search

OWASP-Benchmark / BenchmarkUtils

OWASP Benchmark Project Utilities - Provides scorecard generation and crawling tools for Benchmark style test suites.
https://owasp.org/www-project-benchmark/
GNU General Public License v3.0
16 stars 50 forks source link

Verify correctness of Crawler Attack Success Indicators #4

Closed davewichers closed 3 years ago

davewichers commented 3 years ago

We recently added an optional attack-success-indicator attribute to the sink code block XML files. The intent is that if this string is found in a response, it is in indication of a successful attack, using the attack string. Conversely, this string should not be in a response for a normal attack, otherwise it doesn't discriminate properly. I'd like to enhance the verification crawler to make sure it does discriminate properly.

I think we should build this right into every verification run as follows:

For True Positive test cases:

  1. Submit the normal request and verify the attack-success string is NOT in the response. If not, all is OK. If it is, remember this but keep going.
  2. Submit the attack string and verify the attack-success indicator is in the response. If NOT, then report a test failure as is done now. And then move on.
  3. Only if the attack-success string is in BOTH responses, report a test failure, but indicate that the failure is that the attack-success string is NOT discriminatory. I think it should respond with an explanation something like:

True Positive Test Case X non-discriminatory: The attack-success-string: "THESTRING" was detected in the response returned by both the safe request and the attack request to this test case. To truly verify that a test case is a true positive, the attack-success-string should be unique to the successful attack response, and not be present in a normal response. Please change the attack-success-string and/or the test case sink itself to ensure that the attack-success-string response is present only in a response to a successful attack. [Note: This is a bit wordy, so maybe something shorter? Not that important, as this should be pretty rare.]

For False Positive test case:

I think it is basically the same, with the test logic reversed for Step 2 (which is already implemented), and then in Step 3, if the attack-success-string was in the normal response, it should report that as an error with a message like:

False Positive Test Case X non-discriminatory: The attack-success-string: "THESTRING" was detected in the response returned by the safe request to this test case. To verify that a test case is a false positive, the attack-success-string should not be present in any response to this test case. Please change the attack-success-string and/or the test case sink itself to ensure that the attack-success-string response is present only in a response to a successful attack.

Implementing this should be pretty easy:

Step 1: Update the TESTSUITE-attack-http.xml file generated to add a 'safe' value to the attack parameter.

For example, if the 'attack' parameter looks like: <getparam name="FOO" value="BAR' or '1'='1">, update it to look like: <getparam name="FOO" value="BAR' or '1'='1" safe="foobar">.

Step 2: The Verification crawler then simply issues the:

  1. Safe request first by substituting the 'safe' value for the normal value on the 1 parameter that has this extra info.
  2. The attack request next. Then implement the logic as described above.
davewichers commented 3 years ago

Fixed per merged Pull request.