Skip to content

Commit 0e65a41

Browse files
ErezYalonErezYalon
committed
Update CheckmarxReader.java
Fix error in the reading of Checkmarx results
1 parent d09ba68 commit 0e65a41

File tree

1 file changed

+72
-13
lines changed

1 file changed

+72
-13
lines changed

‎src/main/java/org/owasp/benchmark/score/parsers/CheckmarxReader.java

Lines changed: 72 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public TestResults parse(File f) throws Exception {
4242
Document doc = docBuilder.parse(is);
4343

4444
TestResults tr = new TestResults( "Checkmarx CxSAST",true, TestResults.ToolType.SAST );
45-
45+
4646
// <CxXMLResults DeepLink="http://CHECKMARX2/CxWebClient/ViewerMain.aspx?scanid=52869&amp;projectid=30265"
4747
// ScanStart="Monday, July 27, 2015 4:50:08 PM" Preset="Default 2014" ScanTime="13h:54m:20s"
4848
// LinesOfCodeScanned="1507279" FilesScanned="21075" ReportCreationTime="Tuesday, July 28, 2015 8:38:30 AM"
@@ -51,10 +51,10 @@ public TestResults parse(File f) throws Exception {
5151
Node root = doc.getDocumentElement();
5252
String version = getAttributeValue( "CheckmarxVersion", root );
5353
tr.setToolVersion( version );
54-
54+
5555
String time = getAttributeValue("ScanTime", root);
5656
tr.setTime( time );
57-
57+
5858
List<Node> queryList = getNamedChildren( "Query", root );
5959

6060
for ( Node query : queryList ) {
@@ -68,7 +68,7 @@ public TestResults parse(File f) throws Exception {
6868
}
6969
return tr;
7070
}
71-
71+
7272
private TestCaseResult parseCheckmarxVulnerability(Node query, Node result) {
7373
TestCaseResult tcr = new TestCaseResult();
7474
// <Query id="594" cweId="89" name="SQL_Injection" group="Java_High_Risk" Severity="High"
@@ -80,27 +80,72 @@ private TestCaseResult parseCheckmarxVulnerability(Node query, Node result) {
8080
// state="0" Remark=""
8181
// DeepLink="http://CHECKMARX2/CxWebClient/ViewerMain.aspx?scanid=52869&amp;projectid=30265&amp;pathid=2318"
8282
// SeverityIndex="3">
83-
83+
8484
String cwe = getAttributeValue("cweId", query);
8585
if ( cwe != null ) {
8686
tcr.setCWE( translate( Integer.parseInt(cwe ) ) );
8787
} else {
8888
System.out.println( "flaw: " + query );
8989
}
90-
90+
9191
String name = getAttributeValue("name", query);
9292
tcr.setCategory( name );
9393
// filter out dynamic SQL queries because they report SQL injection separately - these are just dynamic SQL
94-
if ( name.equals( "Dynamic_SQL_Queries" ) ) {
94+
//Other queries are filtered because they are a CHILD_OF of some other query and they share the same cwe id
95+
//We only want the results from the queries that are relevant (PARENT_OF) for the benchmark project
96+
if ( name.equals( "Dynamic_SQL_Queries" ) ||
97+
name.equals( "Heuristic_2nd_Order_SQL_Injection" ) ||
98+
name.equals( "Heuristic_SQL_Injection" ) ||
99+
name.equals( "Second_Order_SQL_Injection" ) ||
100+
name.equals( "Blind_SQL_Injections" ) ||
101+
name.equals( "Improper_Build_Of_Sql_Mapping" ) ||
102+
name.equals( "SQL_Injection_Evasion_Attack" ) ||
103+
name.equals( "Potential_SQL_Injection" ) ||
104+
name.equals( "Client_Side_Injection" ) ||
105+
name.equals( "GWT_DOM_XSS" ) ||
106+
name.equals( "GWT_Reflected_XSS" ) ||
107+
name.equals( "Heuristic_CGI_Stored_XSS" ) ||
108+
name.equals( "Heuristic_Stored_XSS" ) ||
109+
name.equals( "Stored_XSS" ) ||
110+
name.equals( "Suspected_XSS" ) ||
111+
name.equals( "UTF7_XSS" ) ||
112+
name.equals( "CGI_Stored_XSS" ) ||
113+
name.equals( "Potential_GWT_Reflected_XSS" ) ||
114+
name.equals( "Potential_I_Reflected_XSS_All_Clients" ) ||
115+
name.equals( "Potential_IO_Reflected_XSS_All_Clients" ) ||
116+
name.equals( "Potential_O_Reflected_XSS_All_ClientsS" ) ||
117+
name.equals( "Potential_Stored_XSS" ) ||
118+
name.equals( "Potential_UTF7_XSS" ) ||
119+
name.equals( "Stored_Command_Injection" ) ||
120+
name.equals( "CGI_Reflected_XSS_All_Clients" )) {
95121
return null;
96122
}
97123

98-
tcr.setConfidence( Integer.parseInt( getAttributeValue( "SeverityIndex", result) ) );
99-
124+
//In the output xml file from Checkmarx there is no attribute on the node "query" named SeverityIndex
125+
//tcr.setConfidence( Integer.parseInt( getAttributeValue( "SeverityIndex", result) ) );
126+
100127
tcr.setEvidence( getAttributeValue( "name", query ) );
101-
102-
String testcase = getAttributeValue( "FileName", result );
103-
testcase = testcase.substring( testcase.lastIndexOf('/') +1);
128+
129+
/* Some results do not appear in the previous version of this parser because it only look for the attribute "FileName"
130+
* Checkmarx have some results where the input does not start in a BenchmarkTest file so it was necessary to make some changes
131+
* We must consider a good result if the result node "FileName" startsWith BenchmarkTest file or if the last PathNode ends in a "FileName" that startsWith BenchmarkTest file.
132+
* An example is the SeparateClassRequest.java file that have some inputs that we catch but are not been considered as a valid result (FN)
133+
*/
134+
135+
//Get the Path element inside Result
136+
List<Node> paths = getNamedChildren("Path", result);
137+
//Get ALL the PathNodes elements inside each Path
138+
List<Node> pathNodes = getNamedChildren("PathNode", paths.get(0));
139+
//Get the lAST PathNode element from the list above
140+
Node last = pathNodes.get(pathNodes.size() -1);
141+
//Get the FileName element inside the last PathNode
142+
List<Node> fileNames = getNamedChildren( "FileName", last );
143+
Node fileNameNode = fileNames.get(0);
144+
145+
//If the result starts in a BenchmarkTest file
146+
String testcase = getAttributeValue("FileName", result);
147+
//A change was made in the following line due to the paths in the xml outputs file, they are windows based '\\'
148+
testcase = testcase.substring( testcase.lastIndexOf('\\') +1);
104149
if ( testcase.startsWith( "BenchmarkTest" ) ) {
105150
String testno = testcase.substring( "BenchmarkTest".length(), testcase.length() -5 );
106151
try {
@@ -110,7 +155,21 @@ private TestCaseResult parseCheckmarxVulnerability(Node query, Node result) {
110155
}
111156
return tcr;
112157
}
113-
158+
//If not, then the last PastNode must end in a FileName that startsWith BenchmarkTest file
159+
else{
160+
String testcase2 = fileNameNode.getFirstChild().getNodeValue();
161+
testcase2 = testcase2.substring( testcase2.lastIndexOf('\\') +1);
162+
if ( testcase2.startsWith( "BenchmarkTest" ) ) {
163+
String testno2 = testcase2.substring( "BenchmarkTest".length(), testcase2.length() -5 );
164+
try {
165+
tcr.setNumber( Integer.parseInt( testno2 ) );
166+
} catch ( NumberFormatException e ) {
167+
e.printStackTrace();
168+
}
169+
return tcr;
170+
}
171+
}
172+
114173
return null;
115174
}
116175

0 commit comments

Comments
 (0)