search

GaloisInc / BESSPIN-Tool-Suite

The core tool of the BESSPIN Framework.
Other
6 stars 2 forks source link

CWE-761 Scoring Error #1212

Closed njshanahan closed 3 years ago

njshanahan commented 3 years ago

After reviewing test_761.c, I believe that Part 3 does not contain a vulnerability as implemented.

case 3 :
    printf("\n<contains_char_malicious_success> Result is %d\n",contains_char_malicious('S'));

The contains_char_malicious() function will match the first character ('S') within the buffer (i.e. the beginning of the buffer) and free the pointer as expected. The pointer is never incremented, so no violation has occurred. See the test output below.

--------------------Part03: <TEST>--------------------
 cat rm-output.txt

<MATCHED_CHAR> free string and return success

<contains_char_malicious_success> Result is 1
root@ucbvax:~# ./test_761.riscv 3 &> rm-output.txt
root@ucbvax:~#
~~~GDB LOGGING~~~

~~~~~~~~~~~~~~~~~
------------------------------------------------------------

I believe that Part 3 should be reported as NONE, in which case a processor detecting Part 4 would receive credit for mitigating the vulnerability.

|----------|----------|--------------------------------------------------------------------------------------|
| TEST-761 |   HIGH   |                      p01:NONE, p02:NONE, p03:HIGH, p04:DETECTED                      |
|----------|----------|--------------------------------------------------------------------------------------|

Tagging @austinhroach for awareness.

rtadros125 commented 3 years ago

Thanks Nicholas. I am currently working on the partial credit, and I have actually seen the problems with 761. Btw, there are more problems than the what you have identified. For example, part 02 scoring is wrong, and the FreeRTOS scoring is not done well. I am reducing this test to only 2 parts, and I will push these modifications to the partial-credit branch that is still under construction.

I will leave this ticket open till that branch is merged.

njshanahan commented 3 years ago

Thanks @rtadros125. Out of curiosity, what is the problem with Part 2? I'm wondering if our score is correct or if it may change.

rtadros125 commented 3 years ago

@njshanahan here's the commit with the changes: 2530fadbcb364d05cb5cedda4e53b88f0833844e