testable-eu / sast-testability-patterns

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

Update TP: PHP - 47_overriding #37

Open felix-20 opened 1 year ago

felix-20 commented 1 year ago

Testability pattern

47_overriding

Problem statement

The current PHP code:

<?php
class parent_class{

    function F($b){
       echo $b;
    }
}
class child_class extends parent_class{
    function F($b){
        echo "safe";
     }
}
$obj = new parent_class();
$obj->F($_GET['p1']);

This code initializes an object from the parent class, and calls the function F on it. There is also the child_class defined. But why, if only the parent_class is used? I could imagine that this instance wants to test, if the definition of the child_class confuses the tool enough to oversee the vulnerability. However, I would suggest having a second instance here, that actually uses the child class.

Proposed changes

Introduce a second instance to this pattern:

Instance 2 - PHP file:

<?php
class parent_class {
    function F($b) {
       return $b;
    }
}

class child_class extends parent_class {
    function F($b) {
        return "safe";
     }
}

$b = $_GET['p1']; // source
$obj = new child_class();
// The F of child_class is called, so no XSS
$a = $obj->F($b);
echo $a; // sink

The expectation for this instance would be false, but it would make use of the actual overriding.

compaluca commented 1 year ago

I guess we could also create the other two instances:

@felix-20, @enferas : what do you think?