Skip to content

Commit

Permalink
Add 2 new parsers and update/enhance 5 other parsers and add some more
Browse files Browse the repository at this point in the history
test cases for Reader. Also, fix a bug where a classname of Test0042$1,
was being assigned to test case number 0421, rather than 042.
  • Loading branch information
Dave Wichers committed Jan 7, 2025
1 parent 45c883e commit e38ab13
Show file tree
Hide file tree
Showing 9 changed files with 487 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -287,20 +287,6 @@ public String getCategory() {
return this.category;
}

/*
public void setCategory(String category) {
if (Categories.getById(category) != null) {
this.category = category;
} else {
System.out.println(
"ERROR: Unknown vuln category provided to TestCaseResult.setCategory(): "
+ category);
throw new IllegalArgumentException(
"ERROR: Unknown vuln category provided to TestCaseResult.setCategory(): "
+ category);
}
}
*/
public String getEvidence() {
return this.evidence;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/**
* OWASP Benchmark Project
*
* <p>This file is part of the Open Web Application Security Project (OWASP) Benchmark Project For
* details, please see <a
* href="https://owasp.org/www-project-benchmark/">https://owasp.org/www-project-benchmark/</a>.
*
* <p>The OWASP Benchmark is free software: you can redistribute it and/or modify it under the terms
* of the GNU General Public License as published by the Free Software Foundation, version 2.
*
* <p>The OWASP Benchmark is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
* PURPOSE. See the GNU General Public License for more details
*
* @author Dave Wichers
* @created 2025
*/
package org.owasp.benchmarkutils.score.parsers;

import java.util.List;
import org.owasp.benchmarkutils.score.ResultFile;
import org.owasp.benchmarkutils.score.TestCaseResult;
import org.owasp.benchmarkutils.score.TestSuiteResults;
import org.w3c.dom.Node;

/**
* The Cppcheck XML reader parses the XML results file generated when you use the cppcheck analyzer
* to analyze then export results to an XML file. The command is like: cppcheck . --enable=all --xml
* 2> YOURFILENAME.xml. Other parameters are likely required to include support files (-I), or
* possibly exclude certain source files (-i).
*/
public class CppcheckXMLReader extends Reader {

@Override
public boolean canRead(ResultFile resultFile) {
/*
* XML file starts with:
* <?xml version="1.0" encoding="UTF-8"?>
* <results version="2">
* <cppcheck version="2.9" />
* <errors>
* <error id= ...
*/
return resultFile.filename().endsWith(".xml") && resultFile.line(2).contains("cppcheck");
}

@Override
public TestSuiteResults parse(ResultFile resultFile) throws Exception {
TestSuiteResults tr =
new TestSuiteResults("Cppcheck", true, TestSuiteResults.ToolType.SAST);

// get and parse: <cppcheck version="2.9" />
Node cppcheck = getNamedChild("cppcheck", resultFile.xmlRootNode());
String version = getAttributeValue("version", cppcheck);
tr.setToolVersion(version);

Node errorsNode = getNamedChild("errors", resultFile.xmlRootNode());
List<Node> errors = getNamedChildren("error", errorsNode);

for (Node error : errors) {
TestCaseResult tcr = new TestCaseResult();
String filename = getAttributeValue("file0", error);
if (filename == null) {

String ruleid = getAttributeValue("id", error);
switch (ruleid) {
case "ctunullpointer":
case "unusedFunction":
// For these rules, the file location is specified in a child node like
// this: <location
// file="testcases/CWE476_NULL_Pointer_Dereference/CWE476_NULL_Pointer_Dereference__char_41.c" line="28" column="22" info="Dereferencing argument data that is null"/>

Node locationNode = getNamedChild("location", error);
filename = getAttributeValue("file", locationNode);
if (filename == null) {
continue; // Skip to next error Node in for loop since finding is mapped
// to specific file.
}

// Drop through to if (isTestCaseFile(filename)) below
break;

// Check to see if is an 'information' level error like: error:
// id="missingIncludeSystem" severity="information" msg="Include file:
// &lt;stdio.h&gt; not found. Please note: Cppcheck does not need standard
// library headers to get proper results."

// If so, don't generate a warning message
case "checkersReport":
case "missingIncludeSystem": // Had to add
// case "toomanyconfigs": // Have to add --force to command to address
// id="toomanyconfigs" severity="information" msg="Too many #ifdef
// configurations - cppcheck only checks 12 of 19 configurations. Use
// --force to check all configurations."
continue; // Skip to next error Node in for loop

default:
/* System.out.println(
"DRW: Node has no 'file0' attribute: "
+ error
+ " but has ruleid: "
+ ruleid); */
continue; // Skip to next error Node in for loop
}
}

if (isTestCaseFile(filename)) {
tcr.setActualResultTestID(TestSuiteResults.getFileNameNoPath(filename));
String cwe = getAttributeValue("cwe", error);
if (cwe == null) {
String ruleid = getAttributeValue("id", error);
switch (ruleid) {
case "allocaCalled":
// Currently, do nothing. DRW TODO: This should maybe return the CWE for
// buffer overflow, or obsolete function.
// See: "Obsolete function &apos;alloca&apos; called. In C99 and later
// it is recommended to use a variable length array instead."
// verbose="The obsolete function &apos;alloca&apos; is called. In C99
// and later it is recommended to use a variable length array or a
// dynamically allocated array instead. The function &apos;alloca&apos;
// is dangerous for many reasons
// (http://stackoverflow.com/questions/1018853/why-is-alloca-not-considered-good-practice and http://linux.die.net/man/3/alloca).
case "missingOverride":
// Means Override annotation missing when overriding a method.
case "normalCheckLevelMaxBranches": // Limiting analysis of branches. Use
// --check-level=exhaustive to analyze
// all branches
break;
default:
System.err.println(
"WARNING: no CWE value provided for error id: '"
+ ruleid
+ "' file: "
+ TestSuiteResults.getFileNameNoPath(filename));
}
} else {
try {
tcr.setCWE(Integer.parseInt(cwe));
tr.put(tcr);
} catch (NumberFormatException e) {
String ruleid = getAttributeValue("id", error);
System.err.println(
"ERROR: error id: '"
+ ruleid
+ "' has non-integer CWE value of: "
+ cwe);
}
}
} else {
// Do nothing. Skip results for non-test files.
}
}
return tr;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,14 @@ private int figureCWE(
return CweNumber.WEAK_RANDOM;

case "PADDING_ORACLE":
return 326; // Inadequate Encryption Strength - DRW TODO: Does this change Benchmark
// Score
return 326; // Inadequate Encryption Strength

// Weak encryption
case "DES_USAGE": // weak encryption DES
return CweNumber.WEAK_CRYPTO_ALGO;

case "CIPHER_INTEGRITY": // weak encryption - cipher with no integrity
return 353; // Missing support for Integrity Check - DRW TODO: Does this change
// Benchmark Score
return 353; // Missing support for Integrity Check

case "STATIC_IV":
return 329; // static initialization vector for crypto
Expand Down Expand Up @@ -289,8 +287,7 @@ private int figureCWE(
return 478; // Missing Default Case
case "SA_LOCAL_SELF_ASSIGNMENT": // Style: Local Self Assignment
case "UCF_USELESS_CONTROL_FLOW": // Style: Useless Control Flow
return 398; // Code Quality (DRW TODO: also 483, but can't be both. ~same # of
// each.)
return 398; // Code Quality
case "UL_UNRELEASED_LOCK_EXCEPTION_PATH": // MT Correctness:
return 833; // Deadlock
case "UPM_UNCALLED_PRIVATE_METHOD": // Performance: Uncalled Private Method
Expand All @@ -316,6 +313,7 @@ private int figureCWE(
case "RI_REDUNDANT_INTERFACES": // Style:
case "SE_NO_SERIALVERSIONID": // Bad Practice:
case "SE_TRANSIENT_FIELD_NOT_RESTORED": // Bad Practice:
case "SIC_INNER_SHOULD_BE_STATIC": // Performance:
case "SIC_INNER_SHOULD_BE_STATIC_ANON": // Performance:
case "SnVI_NO_SERIALVERSIONID": // Bad Practice:
case "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD": // Style:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,11 @@ private int cweLookup(String vtype, String subtype, Node unifiedNode, String cla
case "Erroneous String Compare":
return 597; // Use of Wrong Operator in String Comparison

case "ToString on Array":
case "Byte Array to String Conversion":
case "Constructor Invokes Overridable Function":
case "Multiple Stream Commits":
case "String Comparison of Float":
case "ToString on Array":
return CweNumber.DONTCARE;

default:
Expand Down Expand Up @@ -475,6 +478,7 @@ private int cweLookup(String vtype, String subtype, Node unifiedNode, String cla

case "Overly Broad Catch":
case "Overly Broad Throws":
case "Throw Inside Finally":
return 703; // Improper Check or Handling of Exceptional Conditions

case "Program Catches NullPointerException":
Expand Down Expand Up @@ -544,12 +548,13 @@ private int cweLookup(String vtype, String subtype, Node unifiedNode, String cla
{
switch (subtype) {
case "":
case "User-Controlled Algorithm":
return CweNumber.WEAK_HASH_ALGO;
case "Missing Required Step":
return 325; // Missing Required Step
default:
System.out.println(
"Fortify parser found vulnerability type: 'Cryptographic Hash', with unmapped subtype: "
"Fortify parser found vulnerability type: 'Weak Cryptographic Hash', with unmapped subtype: "
+ subtype
+ " in class: "
+ classname);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ private String guessCwe(String details) {
case "Weak block mode for Cryptographic Hash Function":
case "Message Digest":
return "328";
case "Cookie without the HttpOnly flag":
return "1004";
case "Base64 Encode":
return "649";
case "Cookie without the HttpOnly flag":
return "1004";
default:
throw new RuntimeException(details);
}
Expand Down
Loading

0 comments on commit e38ab13

Please sign in to comment.