Skip to content

Commit 0c9a3e2

Browse files
dandersonappsecaidandersonappsecai
committed
More changes to the fix verification feature to address feedback from Dave Wichers.
1 parent c657f83 commit 0c9a3e2

File tree

3 files changed

+206
-72
lines changed

3 files changed

+206
-72
lines changed

‎library/src/main/java/org/owasp/benchmarkutils/entities/VerifyFixesOutput.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* OWASP Benchmark Project
3+
*
4+
* <p>This file is part of the Open Web Application Security Project (OWASP) Benchmark Project For
5+
* details, please see <a
6+
* href="https://owasp.org/www-project-benchmark/">https://owasp.org/www-project-benchmark/</a>.
7+
*
8+
* <p>The OWASP Benchmark is free software: you can redistribute it and/or modify it under the terms
9+
* of the GNU General Public License as published by the Free Software Foundation, version 2.
10+
*
11+
* <p>The OWASP Benchmark is distributed in the hope that it will be useful, but WITHOUT ANY
12+
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
13+
* PURPOSE. See the GNU General Public License for more details.
14+
*
15+
* @author David Anderson
16+
* @created 2024
17+
*/
18+
package org.owasp.benchmarkutils.entities;
19+
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
import javax.xml.bind.annotation.XmlRootElement;
23+
24+
@XmlRootElement
25+
public class VerifyFixesOutput {
26+
private List<VerifyFixOutput> list = new ArrayList<>();
27+
28+
public List getList() {
29+
return this.list;
30+
}
31+
32+
public void setList(List list) {
33+
this.list = list;
34+
}
35+
}

‎plugin/src/main/java/org/owasp/benchmarkutils/helpers/Utils.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.owasp.benchmarkutils.entities.ResponseInfo;
6363
import org.owasp.benchmarkutils.entities.TestSuite;
6464
import org.owasp.benchmarkutils.entities.VerifyFixOutput;
65+
import org.owasp.benchmarkutils.entities.VerifyFixesOutput;
6566
import org.owasp.benchmarkutils.tools.TestCaseRequestFileParseException;
6667
import org.owasp.benchmarkutils.tools.TestCaseVerificationResults;
6768
import org.owasp.benchmarkutils.tools.TestCaseVerificationResultsCollection;
@@ -418,7 +419,8 @@ public static String objectToJson(Object object) throws JAXBException {
418419
TestSuite.class,
419420
TestCaseVerificationResults.class,
420421
TestCaseVerificationResultsCollection.class,
421-
VerifyFixOutput.class
422+
VerifyFixOutput.class,
423+
VerifyFixesOutput.class
422424
};
423425
JAXBContext jaxbContext =
424426
org.eclipse.persistence.jaxb.JAXBContextFactory.createContext(

‎plugin/src/main/java/org/owasp/benchmarkutils/tools/BenchmarkCrawlerVerification.java‎

Lines changed: 168 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.owasp.benchmarkutils.entities.TestCaseSetupException;
5858
import org.owasp.benchmarkutils.entities.TestSuite;
5959
import org.owasp.benchmarkutils.entities.VerifyFixOutput;
60+
import org.owasp.benchmarkutils.entities.VerifyFixesOutput;
6061
import org.owasp.benchmarkutils.helpers.Utils;
6162
import org.xml.sax.SAXException;
6263

@@ -116,9 +117,16 @@ public class BenchmarkCrawlerVerification extends BenchmarkCrawler {
116117
private Map<String, TestCaseVerificationResults> testCaseNameToTestCaseVerificationResultsMap =
117118
new HashMap<>();
118119

119-
private List<VerifyFixOutput> exploitedFixedTestcases = new ArrayList<>();
120-
private List<VerifyFixOutput> brokenFixedTestcases = new ArrayList<>();
121-
private List<VerifyFixOutput> notVerifiableFixedTestcases = new ArrayList<>();
120+
private List<VerifyFixOutput> vulnerableTestcases = new ArrayList<>();
121+
private List<VerifyFixOutput> notVulnerableTestcases = new ArrayList<>();
122+
private List<VerifyFixOutput> notVerifiableModifiedVulnerableTestcases = new ArrayList<>();
123+
private List<VerifyFixOutput> exploitedModifiedVulnerableTestcases = new ArrayList<>();
124+
private List<VerifyFixOutput> brokenModifiedVulnerableTestcases = new ArrayList<>();
125+
private List<VerifyFixOutput> notVerifiableModifiedNotVulnerableTestcases = new ArrayList<>();
126+
private List<VerifyFixOutput> exploitedModifiedNotVulnerableTestcases = new ArrayList<>();
127+
private List<VerifyFixOutput> brokenModifiedNotVulnerableTestcases = new ArrayList<>();
128+
129+
private List<VerifyFixOutput> verifyFixesOutputList = new ArrayList<>();
122130

123131
BenchmarkCrawlerVerification() {
124132
// A default constructor required to support Maven plugin API.
@@ -394,10 +402,30 @@ protected void crawl(TestSuite testSuite) throws Exception {
394402
FILE_UNVERIFIABLE_LOG);
395403
}
396404

405+
VerifyFixesOutput verifyFixesOutput = new VerifyFixesOutput();
406+
verifyFixesOutput.setList(verifyFixesOutputList);
407+
408+
File verifyFixResultFile = new File(getOutputDirectory(), FILENAME_VERIFY_FIX_RESULT);
409+
try (BufferedWriter writer = new BufferedWriter(new FileWriter(verifyFixResultFile))) {
410+
String output = Utils.objectToJson(verifyFixesOutput);
411+
// System.out.println(output);
412+
writer.write(output);
413+
} catch (IOException e) {
414+
System.out.println(
415+
"ERROR: Could not write VerifyFixOutputList to file "
416+
+ verifyFixResultFile);
417+
e.printStackTrace();
418+
} catch (JAXBException e) {
419+
System.out.println("ERROR: Could not marshall VerifyFixOutputList to JSON");
420+
e.printStackTrace();
421+
}
422+
397423
System.out.printf("Test case time measurements written to: %s%n", FILE_TIMES_LOG);
398424

399425
RegressionTesting.printCrawlSummary(results);
400-
printFixVerificationSummary();
426+
if (Boolean.parseBoolean(verifyFixed)) {
427+
printFixVerificationSummary();
428+
}
401429
System.out.println();
402430
System.out.println(completionMessage);
403431
}
@@ -406,13 +434,51 @@ protected void crawl(TestSuite testSuite) throws Exception {
406434
// cleanupSetups(setups);
407435
}
408436

437+
private boolean isTestCaseModified(
438+
String unfixedSourceDirectory, String fixedSourceDirectory, String testCaseName)
439+
throws IOException {
440+
// FIXME: Generalize this so it can support languages other than Java and multiple
441+
// source files per testcase.
442+
String unfixedSourceFile =
443+
Paths.get(unfixedSourceDirectory, testCaseName).toString() + ".java";
444+
String fixedSourceFile = Paths.get(fixedSourceDirectory, testCaseName).toString() + ".java";
445+
String unfixedSourceFileContents =
446+
new String(Files.readAllBytes(Paths.get(unfixedSourceFile)));
447+
String fixedSourceFileContents = new String(Files.readAllBytes(Paths.get(fixedSourceFile)));
448+
449+
// Skip testcase in verifyFixed mode if fixed source code is unchanged.
450+
return !unfixedSourceFileContents.equals(fixedSourceFileContents);
451+
}
452+
409453
private void printFixVerificationSummary() {
410-
System.out.println("Fix verification summary:");
411454
System.out.println();
412-
System.out.println("\tExploited fixed test cases:\t" + exploitedFixedTestcases.size());
413-
System.out.println("\tBroken fixed test cases:\t" + brokenFixedTestcases.size());
455+
System.out.println("Fix verification summary");
456+
System.out.println();
457+
System.out.println("Total vulnerable test cases: " + vulnerableTestcases.size());
458+
System.out.println(
459+
"\tProperly fixed (not exploitable):\t"
460+
+ (vulnerableTestcases.size()
461+
- exploitedModifiedVulnerableTestcases.size()));
414462
System.out.println(
415-
"\tNot verifiable fixed test cases:\t" + notVerifiableFixedTestcases.size());
463+
"\tNot correctly fixed (still exploitable):\t"
464+
+ exploitedModifiedVulnerableTestcases.size());
465+
System.out.println(
466+
"\tNot auto-verifiable (can't tell if exploitable):\t"
467+
+ notVerifiableModifiedVulnerableTestcases.size());
468+
System.out.println(
469+
"\tFunctionality broken/modified:\t" + brokenModifiedVulnerableTestcases.size());
470+
System.out.println();
471+
System.out.println("Total not vulnerable test cases: " + notVulnerableTestcases.size());
472+
System.out.println(
473+
"\tStill not exploitable:\t"
474+
+ (notVulnerableTestcases.size()
475+
- exploitedModifiedNotVulnerableTestcases.size()));
476+
System.out.println("\tNow exploitable:\t" + exploitedModifiedNotVulnerableTestcases.size());
477+
System.out.println(
478+
"\tNot auto-verifiable (can't tell if exploitable):\t"
479+
+ notVerifiableModifiedNotVulnerableTestcases.size());
480+
System.out.println(
481+
"\tFunctionality broken/modified:\t" + brokenModifiedNotVulnerableTestcases.size());
416482
}
417483

418484
/**
@@ -538,27 +604,39 @@ protected void handleResponse(TestCaseVerificationResults results)
538604
.getTestCase()
539605
.getName()
540606
.equals(fixedResults.getTestCase().getName())) {
541-
// FIXME: Generalize this so it can support languages other than Java and multiple
542-
// source files per testcase.
543-
String unfixedSourceFile =
544-
Paths.get(unfixedSourceDirectory, unfixedResults.getTestCase().getName())
545-
.toString()
546-
+ ".java";
547-
String fixedSourceFile =
548-
Paths.get(fixedSourceDirectory, fixedResults.getTestCase().getName())
549-
.toString()
550-
+ ".java";
551-
String unfixedSourceFileContents =
552-
new String(Files.readAllBytes(Paths.get(unfixedSourceFile)));
553-
String fixedSourceFileContents =
554-
new String(Files.readAllBytes(Paths.get(fixedSourceFile)));
555-
if (!unfixedSourceFileContents.equals(fixedSourceFileContents)) {
607+
608+
// // FIXME: Generalize this so it can support languages other than Java and
609+
// multiple
610+
// // source files per testcase.
611+
// String unfixedSourceFile =
612+
// Paths.get(unfixedSourceDirectory, unfixedResults.getTestCase().getName())
613+
// .toString()
614+
// + ".java";
615+
// String fixedSourceFile =
616+
// Paths.get(fixedSourceDirectory, fixedResults.getTestCase().getName())
617+
// .toString()
618+
// + ".java";
619+
// String unfixedSourceFileContents =
620+
// new String(Files.readAllBytes(Paths.get(unfixedSourceFile)));
621+
// String fixedSourceFileContents =
622+
// new String(Files.readAllBytes(Paths.get(fixedSourceFile)));
623+
624+
// // Skip testcase in verifyFixed mode if fixed source code is unchanged.
625+
// if (Boolean.parseBoolean(verifyFixed)
626+
// && unfixedSourceFileContents.equals(fixedSourceFileContents)) {
627+
// // System.out.println(
628+
// // "WARNING: Testcase "
629+
// // + fixedResults.getTestCase().getName()
630+
// // + " source file unmodified");
631+
// } else {
632+
// verifyFix(unfixedResults, fixedResults);
633+
// }
634+
if (Boolean.parseBoolean(verifyFixed)
635+
&& isTestCaseModified(
636+
unfixedSourceDirectory,
637+
fixedSourceDirectory,
638+
unfixedResults.getTestCase().getName())) {
556639
verifyFix(unfixedResults, fixedResults);
557-
} else {
558-
System.out.println(
559-
"WARNING: Testcase "
560-
+ fixedResults.getTestCase().getName()
561-
+ " source file unmodified");
562640
}
563641
} else {
564642
System.out.println(
@@ -612,59 +690,78 @@ private TestCaseVerificationResultsCollection loadTestCaseVerificationResults(
612690
}
613691

614692
private boolean verifyFix(
615-
TestCaseVerificationResults beforeFixResults,
616-
TestCaseVerificationResults afterFixResults) {
693+
TestCaseVerificationResults unfixedResults, TestCaseVerificationResults fixedResults) {
694+
695+
// DEBUG
696+
try {
697+
String unfixedResultsJson = Utils.objectToJson(unfixedResults);
698+
System.out.println("unfixedResults JSON: " + unfixedResultsJson);
699+
} catch (Exception e) {
700+
e.printStackTrace();
701+
}
617702

618-
boolean wasNotVerifiable =
619-
afterFixResults.getTestCase().isVulnerability()
620-
&& afterFixResults.getTestCase().isUnverifiable()
621-
&& afterFixResults.isPassed();
703+
boolean isVulnerable = fixedResults.getTestCase().isVulnerability();
704+
// boolean wasNotVerifiable =
705+
// fixedResults.getTestCase().isVulnerability()
706+
// && fixedResults.getTestCase().isUnverifiable()
707+
// && fixedResults.isPassed();
708+
boolean wasNotVerifiable = fixedResults.getTestCase().isUnverifiable();
622709
boolean wasExploited =
623-
afterFixResults.getTestCase().isVulnerability()
624-
&& !afterFixResults.getTestCase().isUnverifiable()
625-
&& afterFixResults.isPassed();
626-
boolean wasBroken =
627-
!beforeFixResults
628-
.getResponseToSafeValue()
629-
.getResponseString()
630-
.equals(afterFixResults.getResponseToSafeValue().getResponseString());
710+
fixedResults.getTestCase().isVulnerability()
711+
&& !fixedResults.getTestCase().isUnverifiable()
712+
&& fixedResults.isPassed();
713+
boolean wasBroken = false;
714+
if (fixedResults.getTestCase().isUnverifiable()) {
715+
wasBroken = false;
716+
} else {
717+
// There will be no safe response info if testcase is not verifiable.
718+
wasBroken =
719+
!unfixedResults
720+
.getResponseToSafeValue()
721+
.getResponseString()
722+
.equals(fixedResults.getResponseToSafeValue().getResponseString());
723+
}
631724

632725
VerifyFixOutput verifyFixOutput = new VerifyFixOutput();
633-
verifyFixOutput.setTestCaseName(afterFixResults.getTestCase().getName());
634-
verifyFixOutput.setUnfixedSafeResponseInfo(beforeFixResults.getResponseToSafeValue());
635-
verifyFixOutput.setUnfixedAttackResponseInfo(beforeFixResults.getResponseToAttackValue());
636-
verifyFixOutput.setFixedSafeResponseInfo(afterFixResults.getResponseToSafeValue());
637-
verifyFixOutput.setFixedAttackResponseInfo(afterFixResults.getResponseToAttackValue());
726+
verifyFixOutput.setTestCaseName(fixedResults.getTestCase().getName());
727+
verifyFixOutput.setUnfixedSafeResponseInfo(unfixedResults.getResponseToSafeValue());
728+
verifyFixOutput.setUnfixedAttackResponseInfo(unfixedResults.getResponseToAttackValue());
729+
verifyFixOutput.setFixedSafeResponseInfo(fixedResults.getResponseToSafeValue());
730+
verifyFixOutput.setFixedAttackResponseInfo(fixedResults.getResponseToAttackValue());
638731
verifyFixOutput.setWasNotVerifiable(wasNotVerifiable);
639732
verifyFixOutput.setWasExploited(wasExploited);
640733
verifyFixOutput.setWasBroken(wasBroken);
641734

642-
if (wasNotVerifiable) {
643-
System.out.println("NOT FIXED: Vulnerability could not be verified");
644-
notVerifiableFixedTestcases.add(verifyFixOutput);
645-
}
646-
if (wasExploited) {
647-
System.out.println("NOT FIXED: Vulnerability was exploited");
648-
exploitedFixedTestcases.add(verifyFixOutput);
649-
}
650-
if (wasBroken) {
651-
System.out.println("NOT FIXED: Functionality was broken");
652-
brokenFixedTestcases.add(verifyFixOutput);
735+
if (isVulnerable) {
736+
vulnerableTestcases.add(verifyFixOutput);
737+
if (wasNotVerifiable) {
738+
System.out.println("NOT FIXED: Vulnerability could not be verified");
739+
notVerifiableModifiedVulnerableTestcases.add(verifyFixOutput);
740+
} else {
741+
if (wasExploited) {
742+
System.out.println("NOT FIXED: Vulnerability was exploited");
743+
exploitedModifiedVulnerableTestcases.add(verifyFixOutput);
744+
}
745+
if (wasBroken) {
746+
System.out.println("NOT FIXED: Functionality was broken");
747+
brokenModifiedVulnerableTestcases.add(verifyFixOutput);
748+
}
749+
}
750+
} else {
751+
notVulnerableTestcases.add(verifyFixOutput);
752+
if (wasNotVerifiable) {
753+
notVerifiableModifiedNotVulnerableTestcases.add(verifyFixOutput);
754+
} else {
755+
if (wasExploited) {
756+
exploitedModifiedNotVulnerableTestcases.add(verifyFixOutput);
757+
}
758+
if (wasBroken) {
759+
brokenModifiedNotVulnerableTestcases.add(verifyFixOutput);
760+
}
761+
}
653762
}
654763

655-
File verifyFixResultFile = new File(getOutputDirectory(), FILENAME_VERIFY_FIX_RESULT);
656-
try (BufferedWriter writer = new BufferedWriter(new FileWriter(verifyFixResultFile))) {
657-
String output = Utils.objectToJson(verifyFixOutput);
658-
// System.out.println(output);
659-
writer.write(output);
660-
} catch (IOException e) {
661-
System.out.println(
662-
"ERROR: Could not write VerifyFixOutput to file " + verifyFixResultFile);
663-
e.printStackTrace();
664-
} catch (JAXBException e) {
665-
System.out.println("ERROR: Could not marshall VerifyFixOutput to JSON");
666-
e.printStackTrace();
667-
}
764+
verifyFixesOutputList.add(verifyFixOutput);
668765

669766
return !wasNotVerifiable && !wasExploited && !wasBroken;
670767
}

0 commit comments

Comments
 (0)