Skip to content

Commit a10332f

Browse files
committed
Add support for SonarQube HotSpot API JSON results. HotSpots are
security specific areas of concern identified by SonarQube.
1 parent e1717d1 commit a10332f

File tree

4 files changed

+156
-29
lines changed

4 files changed

+156
-29
lines changed

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@
301301
-Dcontrast.application.path=/
302302
-Dcontrast.assess.rules.disabled_rules="autocomplete-missing,cache-controls-missing,clickjacking-control-missing,csrf"
303303
-Dcontrast.assess.threshold.entries=100000
304-
-Dcontrast.defend.enabled=false
304+
-Dcontrast.protect.enable=false
305305
-Dcontrast.level=debug
306306
-Dcontrast.log.daily=true
307307
</cargo.jvmargs>

src/main/java/org/owasp/benchmark/score/BenchmarkScore.java

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -705,19 +705,33 @@ else if ( line2 != null && line2.contains("Vendor") && line2.contains("Checkmarx
705705
tr = new SemgrepReader().parse( jsonobj );
706706
} catch (JSONException e) {
707707

708-
try {
709-
jsonobj.getJSONArray("issues");
710-
tr = new SonarQubeJsonReader().parse( fileToParse );
711-
} catch (JSONException e2) {
712-
713-
try {
714-
jsonobj.getJSONArray("issue_events");
715-
tr = new BurpJsonReader().parse( fileToParse );
716-
} catch (JSONException e3) {
708+
// Note: Each of the remaining try blocks is nested under the one above, but we shown them
709+
// inline as they would get too deep otherwise
710+
try {
711+
// SonarQube has two different JSON formats, one for standard issues and
712+
// another for 'hotspots' which are securit issues. Both are handled by the same
713+
// parser for SonarQube.
714+
jsonobj.getJSONArray("issues");
715+
tr = new SonarQubeJsonReader().parse( fileToParse );
716+
} catch (JSONException e2) {
717+
718+
try {
719+
jsonobj.getJSONArray("hotspots");
720+
tr = new SonarQubeJsonReader().parse( fileToParse );
721+
} catch (JSONException e3) {
722+
723+
try {
724+
jsonobj.getJSONArray("issue_events");
725+
tr = new BurpJsonReader().parse( fileToParse );
726+
727+
// This is the final catch that says we couldn't find a matching parser
728+
} catch (JSONException e4) {
717729
System.out.println("Error: No matching parser found for JSON file: " + filename);
718730
}
719-
}
720-
}
731+
732+
} // end catch SonarQubeJsonReader - hotspots
733+
} // end catch SonarQubeJsonReader - issues
734+
} // end catch SemgrepReader
721735
}
722736
}
723737

src/main/java/org/owasp/benchmark/score/parsers/ContrastReader.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Date;
2626

2727
import org.json.JSONObject;
28+
2829
import org.owasp.benchmark.score.BenchmarkScore;
2930

3031
public class ContrastReader extends Reader {
@@ -49,11 +50,11 @@ public TestResults parse(File f) throws Exception {
4950
if (line.startsWith("{\"hash\":")) {
5051
parseContrastFinding(tr, line);
5152
} else if (line.contains("Agent Version:")) {
52-
String version = line.substring(line.indexOf("Version:") + 8);
53+
String version = line.substring(line.indexOf("Version:") + "Version:".length());
5354
tr.setToolVersion(version.trim());
5455
// TODO: expand length of "00001" to match length of TESTCASE_NAME rather than exactly 5
5556
} else if (line.contains("DEBUG - >>> [URL") &&
56-
line.contains(BenchmarkScore.TESTCASENAME+"00001")) {
57+
line.contains(BenchmarkScore.TESTCASENAME+"00001")) {
5758
firstLine = line;
5859
} else if (line.contains("DEBUG - >>> [URL")) {
5960
lastLine = line;
@@ -148,5 +149,4 @@ private String calculateTime(String firstLine, String lastLine) {
148149
}
149150
return null;
150151
}
151-
152152
}

src/main/java/org/owasp/benchmark/score/parsers/SonarQubeJsonReader.java

Lines changed: 127 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,41 @@
3030
public class SonarQubeJsonReader extends Reader {
3131

3232
public TestResults parse( File f ) throws Exception {
33+
34+
TestResults tr = new TestResults( "SonarQube", false, TestResults.ToolType.SAST);
35+
36+
// If the filename includes an elapsed time in seconds (e.g., TOOLNAME-seconds.xml),
37+
// set the compute time on the score card.
38+
tr.setTime(f);
39+
3340
String content = new String(Files.readAllBytes(Paths.get(f.getPath())));
3441

3542
JSONObject obj = new JSONObject(content);
3643
// int version = obj.getInt( "formatVersion" );
3744
JSONArray arr;
45+
46+
boolean hotSpotIssue = true;
47+
48+
// Figure out if there are quality issues or security hotspots in the JSON file
49+
// Each has a different JSON format.
3850
try {
3951
arr = obj.getJSONArray("issues");
52+
hotSpotIssue = false;
4053
} catch (JSONException e) {
41-
System.out.println("ERROR: Couldn't find 'issues' element in SonarQube JSON results."
42-
+ " Maybe not SonarQube results file?" );
43-
return null;
54+
try {
55+
arr = obj.getJSONArray("hotspots");
56+
} catch (JSONException e2) {
57+
System.out.println("ERROR: Couldn't find 'issues' or 'hotspots' element in SonarQube JSON results."
58+
+ " Maybe not SonarQube results file?" );
59+
return null;
60+
}
4461
}
4562

46-
TestResults tr = new TestResults( "SonarJava", false, TestResults.ToolType.SAST);
47-
48-
// If the filename includes an elapsed time in seconds (e.g., TOOLNAME-seconds.xml),
49-
// set the compute time on the score card.
50-
tr.setTime(f);
51-
5263
int numIssues = arr.length();
53-
for (int i = 0; i < numIssues; i++)
54-
{
55-
TestCaseResult tcr = parseSonarQubeFinding( arr.getJSONObject(i) );
64+
for (int i = 0; i < numIssues; i++) {
65+
66+
TestCaseResult tcr = (hotSpotIssue? parseSonarQubeHotSpotIssue( arr.getJSONObject(i) ) :
67+
parseSonarQubeQualityIssue( arr.getJSONObject(i) ));
5668
if ( tcr != null ) {
5769
tr.put( tcr );
5870
}
@@ -61,7 +73,7 @@ public TestResults parse( File f ) throws Exception {
6173
return tr;
6274
}
6375

64-
/**
76+
/** -- Example of Quality Issue JSON object
6577
VULNERABILITY",
6678
"tags":["cwe","owasp-a2","owasp-a6"],
6779
"component":"org.owasp:benchmark:src\/main\/java\/org\/owasp\/benchmark\/testcode\/BenchmarkTest02710.java",
@@ -86,7 +98,10 @@ public TestResults parse( File f ) throws Exception {
8698
8799
**/
88100

89-
private TestCaseResult parseSonarQubeFinding(JSONObject finding ) {
101+
// Quality Issues are normal SonarQube findings that are mostly not relevant to security
102+
// However, there are a small number of security issues that do show up this way so we have
103+
// to support both
104+
private TestCaseResult parseSonarQubeQualityIssue(JSONObject finding ) {
90105
try {
91106
TestCaseResult tcr = new TestCaseResult();
92107
String filename = null;
@@ -116,5 +131,103 @@ private TestCaseResult parseSonarQubeFinding(JSONObject finding ) {
116131
return null;
117132
}
118133

134+
// The parseSonarQubeQualityIssue() method above relies on the SQUID # mapping method in SonarQubeReader.cweLookup()
135+
136+
/** -- Example of HotSpot Issue JSON object
137+
"hotspots": [
138+
{
139+
"key": "AXYEidyZsoEy1bftafT5",
140+
"component": "owasp-benchmark-sonarce:src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00008.java",
141+
"project": "owasp-benchmark-sonarce",
142+
"securityCategory": "sql-injection",
143+
"vulnerabilityProbability": "HIGH",
144+
"status": "TO_REVIEW",
145+
"line": 58,
146+
"message": "Ensure that string concatenation is required and safe for this SQL query.",
147+
"author": "[email protected]",
148+
"creationDate": "2015-08-26T05:13:42+0200",
149+
"updateDate": "2020-11-26T12:53:38+0100"
150+
},
151+
**/
152+
153+
// Hotspot Issues are SonarQube security findings.
154+
private TestCaseResult parseSonarQubeHotSpotIssue(JSONObject finding ) {
155+
try {
156+
TestCaseResult tcr = new TestCaseResult();
157+
String filename = null;
158+
159+
filename = finding.getString("component");
160+
filename = filename.replaceAll( "\\\\", "/"); // In case there are \ instead of / in the path
161+
filename = filename.substring( filename.lastIndexOf( '/' ) );
162+
if ( filename.contains( BenchmarkScore.TESTCASENAME ) ) {
163+
String testNumber = filename.substring( BenchmarkScore.TESTCASENAME.length() + 1,
164+
filename.length() - 5 );
165+
tcr.setNumber( Integer.parseInt( testNumber ) );
166+
String secCat = finding.getString("securityCategory");
167+
if ( secCat == null || secCat.equals("none") ) {
168+
return null;
169+
}
170+
int cwe = securityCategoryCWELookup(secCat, finding.getString("message"));
171+
tcr.setCWE( cwe );
172+
tcr.setCategory( secCat );
173+
tcr.setEvidence( "vulnerabilityProbability: " + finding.getString("vulnerabilityProbability") );
174+
}
175+
176+
return tcr;
177+
} catch (Exception e ) {
178+
e.printStackTrace();
179+
}
180+
return null;
181+
}
182+
183+
/*
184+
* Some of these findings are badly mapped. For example:
185+
* "securityCategory": "xss",
186+
* "message": "Make sure creating this cookie without the \"HttpOnly\" flag is safe.",
187+
* While HttpOnly is a feature to help defend against XSS, it should really be mapped to
188+
* CWE-1004: Sensitive Cookie Without 'HttpOnly' Flag. So we use the 'message' description
189+
* in some findings to move such issues to the 'right' CWE.
190+
* As such, we specifically look at the message in some cases to fix the mapping.
191+
*/
192+
public static int securityCategoryCWELookup(String secCat, String message) {
193+
// Not sure where to look up all the possible security categories in SonarQube, but the mappings
194+
// seem obvious enough.
195+
196+
// Given their horrible mapping scheme, we check each message to detect whether their might be a new
197+
// 'message' mapped to an existing CWE (that might be wrong).
198+
if ( !("Make sure that using this pseudorandom number generator is safe here.".equals(message) ||
199+
"Ensure that string concatenation is required and safe for this SQL query.".equals(message) ||
200+
"Make sure creating this cookie without the \"secure\" flag is safe here.".equals(message) ||
201+
"Make sure that hashing data is safe here.".equals(message) ||
202+
"Make sure creating this cookie without the \"HttpOnly\" flag is safe.".equals(message)) )
203+
{
204+
System.out.println("WARN: Found new SonarQube HotSpot rule not seen before. Category: "
205+
+ secCat + " with message: '" + message + "'");
206+
}
207+
208+
switch( secCat ) {
209+
210+
case "sql-injection" : return 89; // "Ensure that string concatenation is required and safe for this SQL query."
211+
case "insecure-conf" : return 614; // "Make sure creating this cookie without the \"secure\" flag is safe here."
212+
case "xss" : // "Make sure creating this cookie without the \"HttpOnly\" flag is safe."
213+
{
214+
if (message != null && message.contains("HttpOnly")) return 1004;
215+
else return 79; // Actual XSS CWE
216+
}
217+
case "weak-cryptography" : // "Make sure that using this pseudorandom number generator is safe here."
218+
{ // or "Make sure that hashing data is safe here."
219+
if (message != null) {
220+
if (message.contains("pseudorandom")) return 330;
221+
if (message.contains("hashing")) return 328;
222+
else return 0000;
223+
}
224+
else return 327; // Actual Weak Crypto CWE
225+
}
226+
default: System.out.println( "WARN: Failed to translate SonarQube security category: " + secCat );
227+
}
228+
229+
return -1;
230+
}
231+
119232
// This parser relies on the SQUID # mapping method in SonarQubeReader.cweLookup()
120233
}

0 commit comments

Comments
 (0)