Fix scoring math, overflow bug, CWE mappings, and locale bug

plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java

@@ -387,10 +387,7 @@ public static void main(String[] args) {
 
                         SecureRandom generator = SecureRandom.getInstance("SHA1PRNG");
                         while (files.size() > 0) {
-                            int randomNum = generator.nextInt();
-                            // FIXME: Get Absolute Value better
-                            if (randomNum < 0) randomNum *= -1;
-                            int fileToGet = randomNum % files.size();
+                            int fileToGet = generator.nextInt(files.size());
                             File actual = files.remove(fileToGet);
                             // Don't confuse the expected results file as an actual results file if
                             // its in the same directory

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/FaastReader.java

@@ -86,8 +86,7 @@ private TestCaseResult parseFaastFinding(JSONObject finding) {
     }
 
     private String getCategory(String url) {
-        // TODO: Use APPNAME constant rather than 'benchmark' here.
-        String flag = "benchmark/";
+        String flag = BenchmarkScore.TESTSUITENAME.simpleName().toLowerCase() + "/";
         int locator_start = url.lastIndexOf(flag) + flag.length();
         int locator_end = url.lastIndexOf("/" + BenchmarkScore.TESTCASENAME);
         return url.substring(locator_start, locator_end);

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/JuliaReader.java

@@ -20,7 +20,6 @@
 import java.io.StringReader;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
-import org.owasp.benchmarkutils.score.BenchmarkScore;
 import org.owasp.benchmarkutils.score.ResultFile;
 import org.owasp.benchmarkutils.score.TestCaseResult;
 import org.owasp.benchmarkutils.score.TestSuiteResults;
@@ -31,11 +30,6 @@
 
 public class JuliaReader extends Reader {
 
-    // refactoring resilient
-    // TODO: Update to handle package paths from other test suites
-    private final String prefixOfTest =
-            "org.owasp.benchmark.testcode." + BenchmarkScore.TESTCASENAME;
-
     @Override
     public boolean canRead(ResultFile resultFile) {
         return resultFile.filename().endsWith(".xml")

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReader.java

@@ -91,7 +91,8 @@ private int cweLookup(String checkerKey) {
             case "RLK.IN": // Input stream not closed on exit
             case "RLK.OUT": // Output stream not closed on exit
 
-            case "SV.DATA.DB": // Data Injection - what does that mean? TODO
+            case "SV.DATA.DB":
+                return CweNumber.SQL_INJECTION;
             case "SV.PASSWD.HC": // Hardcoded Password
             case "SV.PASSWD.HC.EMPTY": // Empty Password
             case "SV.PASSWD.PLAIN": // Plain-text Password

plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/SemgrepReader.java

@@ -74,8 +74,6 @@ public static int translate(int cwe) {
                 // Output Used by a Downstream Component ('Injection')
             case 75: // CWE vuln mapping DISCOURAGED: Failure to Sanitize Special Elements into a
                 // Different Plane (Special Element Injection)
-            case 77: // Improper Neutralization of Special Elements used in a Command ('Command
-                // Injection') - TODO: Map to Command Injection?
             case 91: // XML Injection (aka Blind XPath Injection)
             case 93: // Improper Neutralization of CRLF Sequences ('CRLF Injection')
             case 94: // Improper Control of Generation of Code ('Code Injection') - Reported when it
@@ -182,8 +180,6 @@ public static int translate(int cwe) {
             case 776: // XEE: Improper Restriction of Recursive Entity References in DTDs ('XML
                 // Entity Expansion')
             case 778: // Insufficient Logging
-            case 780: // Use of RSA Algorithm without OAEP
-                // TODO: Map to Weak Crypto?
             case 787: // Out of bounds Write
             case 798: // Use of Hard-coded Credentials
             case 837: // Improper Enforcement of a Single, Unique Action
@@ -197,16 +193,13 @@ public static int translate(int cwe) {
             case 926: // Improper Export of Android Application Components
             case 939: // Improper Authorization in Handler for Custom URL Scheme
             case 942: // Permissive Cross-domain Policy with Untrusted Domains
-            case 943: // Improper Neutralization of Special Elements in Data Query Logic
-                // TODO: Map this as parent of for various Injection flaw CWEs in Benchmark
             case 1021: // TapJacking: Improper Restriction of Rendered UI Layers or Frames
             case 1104: // Use of Unmaintained Third Party Components
             case 1204: // Generation of Weak Initialization Vector (IV)
             case 1275: // Sensitive Cookie with Improper SameSite Attribute
             case 1323: // Improper Management of Sensitive Trace Data
             case 1333: // Inefficient Regular Expression Complexity (e.g., RegexDOS)
-            case 1336: // Improper Neutralization of Special Elements Used in a Template Engine
-                // TODO: Map to some type of injection?
+            case 1336: // Template Engine Injection -- no Benchmark category exists yet
             case 1390: // Weak Authentication
                 break; // Don't care - So return CWE 'as is'
 
@@ -215,12 +208,14 @@ public static int translate(int cwe) {
             case 23: // Relative Path Traversal
             case 35: // Path Traversal: '.../...//'
                 return CweNumber.PATH_TRAVERSAL;
+            case 77: // Command Injection parent CWE
             case 78:
                 return CweNumber.COMMAND_INJECTION;
             case 79:
             case 80: // Basic XSS
                 return CweNumber.XSS;
             case 89:
+            case 943: // Data Query Logic injection
                 return CweNumber.SQL_INJECTION;
             case 90:
                 return CweNumber.LDAP_INJECTION;
@@ -229,6 +224,7 @@ public static int translate(int cwe) {
             case 329: // Generation of Predictable IV with CBC Mode - Has no affect on Benchmark -
                 // but leaving mapping in anyway
             case 696: // Incorrect Behavior Order
+            case 780: // RSA without OAEP
                 return CweNumber.WEAK_CRYPTO_ALGO; // weak encryption
             case 328:
                 return CweNumber.WEAK_HASH_ALGO;

plugin/src/main/java/org/owasp/benchmarkutils/score/report/Formats.java

@@ -18,11 +18,19 @@
 package org.owasp.benchmarkutils.score.report;
 
 import java.text.DecimalFormat;
+import java.text.DecimalFormatSymbols;
+import java.util.Locale;
 
 public class Formats {
 
-    public static final DecimalFormat twoDecimalPlacesPercentage = new DecimalFormat("#0.00%");
+    private static final DecimalFormatSymbols US_SYMBOLS =
+            DecimalFormatSymbols.getInstance(Locale.US);
 
-    public static final DecimalFormat singleDecimalPlaceNumber = new DecimalFormat("0.0");
-    public static final DecimalFormat fourDecimalPlacesNumber = new DecimalFormat("#0.0000");
+    public static final DecimalFormat twoDecimalPlacesPercentage =
+            new DecimalFormat("#0.00%", US_SYMBOLS);
+
+    public static final DecimalFormat singleDecimalPlaceNumber =
+            new DecimalFormat("0.0", US_SYMBOLS);
+    public static final DecimalFormat fourDecimalPlacesNumber =
+            new DecimalFormat("#0.0000", US_SYMBOLS);
 }

plugin/src/main/java/org/owasp/benchmarkutils/score/report/ScatterVulns.java

@@ -341,10 +341,10 @@ private void makeLegend(
                 if (ch == 'Z') ch = 'a';
                 else ch++;
 
-                noncommercialTotalScore += or.getCategoryResults(category).score;
-                noncommercialTotalPrecision += or.getCategoryResults(category).precision;
-                noncommercialTotalTPR += or.getCategoryResults(category).truePositiveRate;
-                noncommercialTotalFPR += or.getCategoryResults(category).falsePositiveRate;
+                noncommercialTotalScore += or.getCategoryResults(category).score * 100;
+                noncommercialTotalPrecision += or.getCategoryResults(category).precision * 100;
+                noncommercialTotalTPR += or.getCategoryResults(category).truePositiveRate * 100;
+                noncommercialTotalFPR += or.getCategoryResults(category).falsePositiveRate * 100;
 
                 double score = or.getCategoryResults(category).score * 100;
                 if (score < noncommercialLow) {

plugin/src/main/java/org/owasp/benchmarkutils/score/report/ToolReport.java

@@ -166,8 +166,6 @@ else if (r.truePositiveRate > .7 && r.falsePositiveRate < .3)
                 CategoryResults currentCategoryResults = overallAveToolResults.get(categoryName);
                 // default value hard spaces equal to triangle width
                 String precisionBonus = "&nbsp;&nbsp;&nbsp;&nbsp;";
-                // r.precision has range 0-1, but currentCategoryResults.precision is 1 to 100.
-                // FIXME: Fix precision calculations so they are the same units
                 double precisionDiff = 100 * r.precision - currentCategoryResults.precision;
                 if (precisionDiff >= 5)
                     precisionBonus =
@@ -184,7 +182,6 @@ else if (precisionDiff <= -5) {
 
                 // default value hard spaces equal to triangle width
                 String fscoreBonus = "&nbsp;&nbsp;&nbsp;&nbsp;";
-                // FIXME: Fix fscore calculations so they are the same units
                 double fscoreDiff = 100 * r.fscore - currentCategoryResults.fscore;
                 if (fscoreDiff >= 5) fscoreBonus = "<span style=\"color: green\">&#9650;</span>";
                 else if (fscoreDiff <= -5) {
@@ -198,7 +195,6 @@ else if (fscoreDiff <= -5) {
 
                 // default value hard spaces equal to triangle width
                 recallBonus = "&nbsp;&nbsp;&nbsp;&nbsp;";
-                // FIXME: Fix truePositiveRate calculations so they are the same units
                 double recallDiff =
                         100 * r.truePositiveRate - currentCategoryResults.truePositiveRate;
                 if (recallDiff >= 5) recallBonus = "<span style=\"color: green\">&#9650;</span>";

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReaderTest.java

@@ -52,9 +52,10 @@ void readerHandlesGivenResultFile() throws Exception {
         assertTrue(result.isCommercial());
         assertEquals("Klocwork", result.getToolName());
 
-        assertEquals(2, result.getTotalResults());
+        assertEquals(3, result.getTotalResults());
 
         assertEquals(CweNumber.SQL_INJECTION, result.get(1).get(0).getCWE());
         assertEquals(CweNumber.PATH_TRAVERSAL, result.get(2).get(0).getCWE());
+        assertEquals(CweNumber.SQL_INJECTION, result.get(3).get(0).getCWE());
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/SemgrepReaderTest.java

@@ -76,9 +76,44 @@ void readerHandlesGivenResultFileInV121() throws Exception {
         assertFalse(result.isCommercial());
         assertEquals("Semgrep", result.getToolName());
 
-        assertEquals(2, result.getTotalResults());
+        assertEquals(5, result.getTotalResults());
 
         assertEquals(CweNumber.COMMAND_INJECTION, result.get(3).get(0).getCWE());
         assertEquals(CweNumber.COOKIE_WITHOUT_HTTPONLY, result.get(4).get(0).getCWE());
+        assertEquals(CweNumber.COMMAND_INJECTION, result.get(5).get(0).getCWE());
+        assertEquals(CweNumber.WEAK_CRYPTO_ALGO, result.get(6).get(0).getCWE());
+        assertEquals(CweNumber.SQL_INJECTION, result.get(7).get(0).getCWE());
+    }
+
+    @Test
+    void translateCwe77ReturnsCommandInjection() {
+        assertEquals(
+                CweNumber.COMMAND_INJECTION,
+                SemgrepReader.translate(77),
+                "CWE-77 (Command Injection parent) should map to 78 (COMMAND_INJECTION)");
+    }
+
+    @Test
+    void translateCwe780ReturnsWeakCrypto() {
+        assertEquals(
+                CweNumber.WEAK_CRYPTO_ALGO,
+                SemgrepReader.translate(780),
+                "CWE-780 (RSA without OAEP) should map to 327 (WEAK_CRYPTO_ALGO)");
+    }
+
+    @Test
+    void translateCwe943ReturnsSqlInjection() {
+        assertEquals(
+                CweNumber.SQL_INJECTION,
+                SemgrepReader.translate(943),
+                "CWE-943 (Data Query Injection) should map to 89 (SQL_INJECTION)");
+    }
+
+    @Test
+    void translateDontCareCwesReturnAsIs() {
+        assertEquals(75, SemgrepReader.translate(75), "CWE-75 should pass through unchanged");
+        assertEquals(778, SemgrepReader.translate(778), "CWE-778 should pass through unchanged");
+        assertEquals(942, SemgrepReader.translate(942), "CWE-942 should pass through unchanged");
+        assertEquals(502, SemgrepReader.translate(502), "CWE-502 should pass through unchanged");
     }
 }

plugin/src/test/java/org/owasp/benchmarkutils/score/report/FormatsTest.java

@@ -22,10 +22,25 @@
 import static org.owasp.benchmarkutils.score.report.Formats.singleDecimalPlaceNumber;
 import static org.owasp.benchmarkutils.score.report.Formats.twoDecimalPlacesPercentage;
 
+import java.util.Locale;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 class FormatsTest {
 
+    private Locale originalLocale;
+
+    @BeforeEach
+    void saveLocale() {
+        originalLocale = Locale.getDefault();
+    }
+
+    @AfterEach
+    void restoreLocale() {
+        Locale.setDefault(originalLocale);
+    }
+
     @Test
     void hasFormatterForTwoDecimalPlacesPercentage() {
         assertEquals("1234.57%", twoDecimalPlacesPercentage.format(12.345678));
@@ -40,4 +55,22 @@ void hasFormatterForFourDecimalPlaces() {
     void hasFormatterForSingleDecimalPlace() {
         assertEquals("12.3", singleDecimalPlaceNumber.format(12.345678));
     }
+
+    @Test
+    void formatsUseDotDecimalSeparatorRegardlessOfLocale() {
+        Locale.setDefault(Locale.GERMANY);
+
+        assertEquals(
+                "1234.57%",
+                twoDecimalPlacesPercentage.format(12.345678),
+                "Percentage formatter must use dot separator even under German locale");
+        assertEquals(
+                "12.3",
+                singleDecimalPlaceNumber.format(12.345678),
+                "Single decimal formatter must use dot separator even under German locale");
+        assertEquals(
+                "12.3457",
+                fourDecimalPlacesNumber.format(12.345678),
+                "Four decimal formatter must use dot separator even under German locale");
+    }
 }

plugin/src/test/resources/testfiles/Benchmark_Klocwork.csv

@@ -1,3 +1,4 @@
 File,Path,Line,Method,Code,Severity,State,Status,Taxonomy,Owner
 BenchmarkTest00001.java,/opt/klocwork/projects_root/projects/BenchmarkJavaToo/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00001.java,58,doPost(),SV.SQL,Error (2),New,Analyze,Java,unowned
-BenchmarkTest00002.java,/opt/klocwork/projects_root/projects/BenchmarkJavaToo/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00002.java,64,doPost(),SV.PATH,Warning (3),New,Analyze,Java,unowned
+BenchmarkTest00002.java,/opt/klocwork/projects_root/projects/BenchmarkJavaToo/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00002.java,64,doPost(),SV.PATH,Warning (3),New,Analyze,Java,unowned
+BenchmarkTest00003.java,/opt/klocwork/projects_root/projects/BenchmarkJavaToo/src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00003.java,58,doPost(),SV.DATA.DB,Error (2),New,Analyze,Java,unowned

plugin/src/test/resources/testfiles/Benchmark_semgrep-v0.121.0.json

@@ -196,6 +196,84 @@
         "line": 42,
         "offset": 1802
       }
+    },
+    {
+      "check_id": "java.lang.security.audit.command-injection-general",
+      "end": { "col": 4, "line": 62, "offset": 2300 },
+      "extra": {
+        "fingerprint": "aaa111",
+        "is_ignored": false,
+        "lines": "Runtime.exec(cmd)",
+        "message": "Command injection finding",
+        "metadata": {
+          "category": "security",
+          "confidence": "LOW",
+          "cwe": [
+            "CWE-77: Improper Neutralization of Special Elements used in a Command"
+          ],
+          "impact": "HIGH",
+          "likelihood": "LOW",
+          "owasp": [
+            "A03:2021 - Injection"
+          ]
+        },
+        "metavars": {},
+        "severity": "WARNING"
+      },
+      "path": "src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00005.java",
+      "start": { "col": 3, "line": 48, "offset": 2000 }
+    },
+    {
+      "check_id": "java.lang.security.audit.rsa-no-oaep",
+      "end": { "col": 4, "line": 62, "offset": 2300 },
+      "extra": {
+        "fingerprint": "bbb222",
+        "is_ignored": false,
+        "lines": "Cipher.getInstance(RSA)",
+        "message": "RSA without OAEP finding",
+        "metadata": {
+          "category": "security",
+          "confidence": "LOW",
+          "cwe": [
+            "CWE-780: Use of RSA Algorithm without OAEP"
+          ],
+          "impact": "HIGH",
+          "likelihood": "LOW",
+          "owasp": [
+            "A02:2021 - Cryptographic Failures"
+          ]
+        },
+        "metavars": {},
+        "severity": "WARNING"
+      },
+      "path": "src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00006.java",
+      "start": { "col": 3, "line": 48, "offset": 2000 }
+    },
+    {
+      "check_id": "java.lang.security.audit.data-query-injection",
+      "end": { "col": 4, "line": 62, "offset": 2300 },
+      "extra": {
+        "fingerprint": "ccc333",
+        "is_ignored": false,
+        "lines": "query.filter(input)",
+        "message": "Data query injection finding",
+        "metadata": {
+          "category": "security",
+          "confidence": "LOW",
+          "cwe": [
+            "CWE-943: Improper Neutralization of Special Elements in Data Query Logic"
+          ],
+          "impact": "HIGH",
+          "likelihood": "LOW",
+          "owasp": [
+            "A03:2021 - Injection"
+          ]
+        },
+        "metavars": {},
+        "severity": "WARNING"
+      },
+      "path": "src/main/java/org/owasp/benchmark/testcode/BenchmarkTest00007.java",
+      "start": { "col": 3, "line": 48, "offset": 2000 }
     }
   ],
   "version": "0.121.0"