testable-eu / sast-testability-patterns

Testability Pattern Catalogs for SAST
https://owasp.org/www-project-testability-patterns-for-web-applications/
Apache License 2.0
29 stars 2 forks source link

Update TP: PHP - 75_buffer #38

Open felix-20 opened 1 year ago

felix-20 commented 1 year ago

Testability pattern

75_buffer

Problem statement

There is not really a problem with the current pattern, this is just a suggestion for a new instance. According to this there is the function ob_get_clean(), that executes executes both ob_get_contents() and ob_end_clean(). That function is not yet captured by this pattern.

Proposed changes

Introduce a new instance with the following code:

$a = $_GET["p1"]; // source
ob_start();
echo $a;
$x = ob_get_clean();
echo $x; // sink

It is very similar to the first instance, but it only uses one function instead of two.

compaluca commented 1 year ago

@felix-20, @enferas : do we need to keep the original instance? Are you considering that the SAST tool may not have some of these functions properly modeled?

enferas commented 1 year ago

The problem here that static tools will not report the correct sink. because they will just ignore the buffer.

In this code,

$a = $_GET["p1"]; // source
ob_start();
echo $a;
$x = ob_get_clean();
echo $x; // sink

static tools will report echo $a but actually the correct sink is echo $x.

It is a different case than other patterns because in this pattern we have the alert but not accurate.

I am not sure if we want to delete it or mark it as a special pattern.