Skip to content

Commit 9d34898

Browse files
goneallgoneall
committed
Fix issues identified in Sonatype lift
Issues include: - Securing against bad redirects for the spdx.org/licenses site - Updating the log4j logging version - Specifying UTF-8 character sets - Making ChecksumAlgorithm more immutable - Fixing potential race conditions Signed-off-by: Gary O'Neall <[email protected]>
1 parent 2e20d42 commit 9d34898

File tree

6 files changed

+65
-39
lines changed

6 files changed

+65
-39
lines changed

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@
9393
<dependency>
9494
<groupId>org.apache.logging.log4j</groupId>
9595
<artifactId>log4j-slf4j-impl</artifactId>
96-
<version>2.17.0</version>
96+
<version>2.17.1</version>
9797
</dependency>
9898
<dependency>
9999
<groupId>org.apache.commons</groupId>

‎src/main/java/org/spdx/library/DefaultModelStore.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ private DefaultModelStore() {
4444
}
4545

4646
public static IModelStore getDefaultModelStore() {
47-
return defaultModelStore;
47+
lock.readLock().lock();
48+
try {
49+
return defaultModelStore;
50+
} finally {
51+
lock.readLock().unlock();
52+
}
4853
}
4954

5055
public static String getDefaultDocumentUri() {

‎src/main/java/org/spdx/library/model/enumerations/ChecksumAlgorithm.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public enum ChecksumAlgorithm implements IndividualUriValue {
4747
ADLER32("checksumAlgorithm_adler32"),
4848
;
4949

50-
private String longName;
50+
private final String longName;
5151

5252
private ChecksumAlgorithm(String longName) {
5353
this.longName = longName;

‎src/main/java/org/spdx/library/referencetype/ListedReferenceTypes.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,15 @@ private void loadReferenceTypeNames() {
112112
* @return the listed reference types as maintained by the SPDX workgroup
113113
*/
114114
public static ListedReferenceTypes getListedReferenceTypes() {
115-
if (listedReferenceTypes == null) {
116-
listedReferenceTypesModificationLock.writeLock().lock();
117-
try {
118-
if (listedReferenceTypes == null) {
119-
listedReferenceTypes = new ListedReferenceTypes();
120-
}
121-
} finally {
122-
listedReferenceTypesModificationLock.writeLock().unlock();
115+
listedReferenceTypesModificationLock.writeLock().lock();
116+
try {
117+
if (listedReferenceTypes == null) {
118+
listedReferenceTypes = new ListedReferenceTypes();
123119
}
120+
return listedReferenceTypes;
121+
} finally {
122+
listedReferenceTypesModificationLock.writeLock().unlock();
124123
}
125-
return listedReferenceTypes;
126124
}
127125

128126
/**

‎src/main/java/org/spdx/storage/listedlicense/SpdxListedLicenseModelStore.java‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ private void loadIds() throws InvalidSPDXAnalysisException {
150150
try {
151151
// read the license IDs
152152
tocStream = getTocInputStream();
153-
reader = new BufferedReader(new InputStreamReader(tocStream));
153+
reader = new BufferedReader(new InputStreamReader(tocStream, "UTF-8"));
154154
StringBuilder tocJsonStr = new StringBuilder();
155155
String line;
156156
while((line = reader.readLine()) != null) {
@@ -162,17 +162,17 @@ private void loadIds() throws InvalidSPDXAnalysisException {
162162

163163
// read the exception ID's
164164
tocStream = getExceptionTocInputStream();
165-
reader = new BufferedReader(new InputStreamReader(tocStream));
165+
reader = new BufferedReader(new InputStreamReader(tocStream, "UTF-8"));
166166
tocJsonStr = new StringBuilder();
167167
while((line = reader.readLine()) != null) {
168168
tocJsonStr.append(line);
169169
}
170170
ExceptionJsonTOC exceptionToc = gson.fromJson(tocJsonStr.toString(), ExceptionJsonTOC.class);
171171
exceptionIds = exceptionToc.getExceptionIds();
172172
} catch (MalformedURLException e) {
173-
throw(new SpdxListedLicenseException("License TOC URL invalid"));
173+
throw new SpdxListedLicenseException("License TOC URL invalid") ;
174174
} catch (IOException e) {
175-
throw(new SpdxListedLicenseException("I/O error reading license TOC"));
175+
throw new SpdxListedLicenseException("I/O error reading license TOC");
176176
} finally {
177177
if (reader != null) {
178178
try {
@@ -195,9 +195,14 @@ public boolean exists(String documentUri, String id) {
195195
if (!SpdxConstants.LISTED_LICENSE_URL.equals(documentUri)) {
196196
return false;
197197
}
198+
listedLicenseModificationLock.readLock().lock();
199+
try {
198200
return this.licenseIds.containsKey(id.toLowerCase()) ||
199201
this.exceptionIds.containsKey(id.toLowerCase()) ||
200202
this.crossRefs.containsKey(id);
203+
} finally {
204+
listedLicenseModificationLock.readLock().unlock();
205+
}
201206
}
202207

203208
/* (non-Javadoc)
@@ -260,21 +265,21 @@ public List<String> getPropertyValueNames(String documentUri, String id) throws
260265
} else if (crossRefs.containsKey(id)) {
261266
crossRef = crossRefs.get(id);
262267
}
268+
if (isLicenseId) {
269+
LicenseJson license = fetchLicenseJson(licenseIds.get(id.toLowerCase()));
270+
return license.getPropertyValueNames();
271+
} else if (isExceptionId) {
272+
ExceptionJson exc = fetchExceptionJson(exceptionIds.get(id.toLowerCase()));
273+
return exc.getPropertyValueNames();
274+
} else if (Objects.nonNull(crossRef)) {
275+
return crossRef.getPropertyValueNames();
276+
} else {
277+
logger.error("ID "+id+" is not a listed license ID, crossRef ID nor a listed exception ID");
278+
throw new SpdxIdNotFoundException("ID "+id+" is not a listed license ID. crossRef ID nor a listed exception ID");
279+
}
263280
} finally {
264281
listedLicenseModificationLock.readLock().unlock();
265282
}
266-
if (isLicenseId) {
267-
LicenseJson license = fetchLicenseJson(licenseIds.get(id.toLowerCase()));
268-
return license.getPropertyValueNames();
269-
} else if (isExceptionId) {
270-
ExceptionJson exc = fetchExceptionJson(exceptionIds.get(id.toLowerCase()));
271-
return exc.getPropertyValueNames();
272-
} else if (Objects.nonNull(crossRef)) {
273-
return crossRef.getPropertyValueNames();
274-
} else {
275-
logger.error("ID "+id+" is not a listed license ID, crossRef ID nor a listed exception ID");
276-
throw new SpdxIdNotFoundException("ID "+id+" is not a listed license ID. crossRef ID nor a listed exception ID");
277-
}
278283
}
279284

280285
/**
@@ -322,10 +327,10 @@ private LicenseJson fetchLicenseJson(String idCaseInsensitive) throws InvalidSPD
322327
this.listedLicenseCache.put(id, license);
323328
} catch (MalformedURLException e) {
324329
logger.error("Json license invalid for ID "+id);
325-
throw(new SpdxListedLicenseException("JSON license URL invalid for ID "+id));
330+
throw new SpdxListedLicenseException("JSON license URL invalid for ID "+id);
326331
} catch (IOException e) {
327332
logger.error("I/O error opening Json license URL");
328-
throw(new SpdxListedLicenseException("I/O Error reading license data for ID "+id));
333+
throw new SpdxListedLicenseException("I/O Error reading license data for ID "+id);
329334
} finally {
330335
if (reader != null) {
331336
try {
@@ -393,10 +398,10 @@ private ExceptionJson fetchExceptionJson(String idCaseInsensitive) throws Invali
393398
this.listedExceptionCache.put(id, exc);
394399
} catch (MalformedURLException e) {
395400
logger.error("Json license invalid for ID "+id);
396-
throw(new SpdxListedLicenseException("JSON license URL invalid for ID "+id));
401+
throw new SpdxListedLicenseException("JSON license URL invalid for ID "+id);
397402
} catch (IOException e) {
398403
logger.error("I/O error opening Json license URL");
399-
throw(new SpdxListedLicenseException("I/O Error reading license data for ID "+id));
404+
throw new SpdxListedLicenseException("I/O Error reading license data for ID "+id);
400405
} finally {
401406
if (reader != null) {
402407
try {
@@ -663,8 +668,8 @@ public Object next() {
663668
throw new RuntimeException(new InvalidSPDXAnalysisException("Invalid type for "+propertyName+". Must be of type CrossRefJson"));
664669
}
665670
CrossRefJson nextCrossRef = (CrossRefJson)nextVal;
666-
listedLicenseModificationLock.writeLock().lock();
667671
String crossRefId = nextCrossRef.getId();
672+
listedLicenseModificationLock.writeLock().lock();
668673
try {
669674
if (Objects.isNull(crossRefId)) {
670675
// Need to create an ID and store it in the cache
@@ -977,7 +982,7 @@ public boolean collectionContains(String documentUri, String id, String property
977982
if (isLicenseId) {
978983
LicenseJson license = fetchLicenseJson(id);
979984
List<Object> valueList = (List<Object>)(List<?>)license.getValueList(propertyName);
980-
if (value instanceof TypedValue && (SpdxConstants.CLASS_CROSS_REF.equals(((TypedValue)value).getType()))) {
985+
if (value instanceof TypedValue && SpdxConstants.CLASS_CROSS_REF.equals(((TypedValue)value).getType())) {
981986
CrossRefJson compareValue = crossRefs.get(((TypedValue)value).getId());
982987
if (Objects.isNull(compareValue)) {
983988
return false;
@@ -1179,7 +1184,7 @@ public void delete(String documentUri, String id) throws InvalidSPDXAnalysisExce
11791184
throw new SpdxIdNotFoundException("Document URI for SPDX listed licenses is expected to be "+
11801185
SpdxConstants.LISTED_LICENSE_URL + ". Supplied document URI was "+documentUri);
11811186
}
1182-
listedLicenseModificationLock.writeLock().lock();;
1187+
listedLicenseModificationLock.writeLock().lock();
11831188
try {
11841189
if (licenseIds.containsKey(id.toLowerCase())) {
11851190
this.listedLicenseCache.remove(id);

‎src/main/java/org/spdx/storage/listedlicense/SpdxListedLicenseWebStore.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
import java.io.InputStream;
2222
import java.net.HttpURLConnection;
2323
import java.net.URL;
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.List;
2427
import java.util.Objects;
2528

2629
import org.spdx.library.InvalidSPDXAnalysisException;
@@ -33,6 +36,9 @@
3336
public class SpdxListedLicenseWebStore extends SpdxListedLicenseModelStore {
3437

3538
private static final int READ_TIMEOUT = 5000;
39+
40+
static final List<String> WHITE_LIST = Collections.unmodifiableList(Arrays.asList(
41+
"spdx.org", "spdx.dev", "spdx.com", "spdx.info")); // Allowed host names for the SPDX listed licenses
3642

3743
/**
3844
* @throws InvalidSPDXAnalysisException
@@ -49,11 +55,23 @@ private InputStream getUrlInputStream(URL url) throws IOException {
4955
(status == HttpURLConnection.HTTP_MOVED_TEMP || status == HttpURLConnection.HTTP_MOVED_PERM
5056
|| status == HttpURLConnection.HTTP_SEE_OTHER)) {
5157
// redirect
52-
String redirectUrl = connection.getHeaderField("Location");
53-
if (Objects.isNull(redirectUrl) || redirectUrl.isEmpty()) {
54-
throw new IOException("Invalid redirect URL response");
58+
String redirectUrlStr = connection.getHeaderField("Location");
59+
if (Objects.isNull(redirectUrlStr) || redirectUrlStr.isEmpty()) {
60+
throw new IOException("Empty redirect URL response");
61+
}
62+
URL redirectUrl;
63+
try {
64+
redirectUrl = new URL(redirectUrlStr);
65+
} catch(Exception ex) {
66+
throw new IOException("Invalid redirect URL");
67+
}
68+
if (!redirectUrl.getProtocol().toLowerCase().startsWith("http")) {
69+
throw new IOException("Invalid redirect protocol");
70+
}
71+
if (!WHITE_LIST.contains(redirectUrl.getHost())) {
72+
throw new IOException("Invalid redirect host - not on the allowed 'white list'");
5573
}
56-
connection = (HttpURLConnection)new URL(redirectUrl).openConnection();
74+
connection = (HttpURLConnection)redirectUrl.openConnection();
5775
}
5876
return connection.getInputStream();
5977
}

0 commit comments

Comments
 (0)