diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java index ca832fad..769a3cd7 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/BenchmarkScore.java +++ b/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 diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/FaastReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/FaastReader.java index ebbad18a..da9e1f4e 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/FaastReader.java +++ b/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); diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/JuliaReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/JuliaReader.java index c18df280..9f999fd9 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/JuliaReader.java +++ b/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") diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReader.java index 54c4145c..f6d80e80 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReader.java +++ b/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 diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/SemgrepReader.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/SemgrepReader.java index 9a3dd4e6..be369095 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/parsers/SemgrepReader.java +++ b/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; diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/Formats.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/report/Formats.java index a3bf06e8..969bb66e 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/Formats.java +++ b/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); } diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ScatterVulns.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ScatterVulns.java index cd511a03..6c61248f 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ScatterVulns.java +++ b/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) { diff --git a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ToolReport.java b/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ToolReport.java index 9f634f44..2c0f71b3 100644 --- a/plugin/src/main/java/org/owasp/benchmarkutils/score/report/ToolReport.java +++ b/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 = "    "; - // 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 = "    "; - // FIXME: Fix fscore calculations so they are the same units double fscoreDiff = 100 * r.fscore - currentCategoryResults.fscore; if (fscoreDiff >= 5) fscoreBonus = "▲"; else if (fscoreDiff <= -5) { @@ -198,7 +195,6 @@ else if (fscoreDiff <= -5) { // default value hard spaces equal to triangle width recallBonus = "    "; - // FIXME: Fix truePositiveRate calculations so they are the same units double recallDiff = 100 * r.truePositiveRate - currentCategoryResults.truePositiveRate; if (recallDiff >= 5) recallBonus = "▲"; diff --git a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReaderTest.java b/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReaderTest.java index 5f9f91df..87c8f997 100644 --- a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/KlocworkCSVReaderTest.java +++ b/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()); } } diff --git a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/SemgrepReaderTest.java b/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/SemgrepReaderTest.java index 97f2537c..b51d3a28 100644 --- a/plugin/src/test/java/org/owasp/benchmarkutils/score/parsers/SemgrepReaderTest.java +++ b/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"); } } diff --git a/plugin/src/test/java/org/owasp/benchmarkutils/score/report/FormatsTest.java b/plugin/src/test/java/org/owasp/benchmarkutils/score/report/FormatsTest.java index af569169..7abac7ac 100644 --- a/plugin/src/test/java/org/owasp/benchmarkutils/score/report/FormatsTest.java +++ b/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"); + } } diff --git a/plugin/src/test/resources/testfiles/Benchmark_Klocwork.csv b/plugin/src/test/resources/testfiles/Benchmark_Klocwork.csv index ee84160b..b7d97172 100644 --- a/plugin/src/test/resources/testfiles/Benchmark_Klocwork.csv +++ b/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 diff --git a/plugin/src/test/resources/testfiles/Benchmark_semgrep-v0.121.0.json b/plugin/src/test/resources/testfiles/Benchmark_semgrep-v0.121.0.json index b22fabd4..e493798c 100644 --- a/plugin/src/test/resources/testfiles/Benchmark_semgrep-v0.121.0.json +++ b/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"