Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Not resolved instanceof property #4

Closed
eapunk opened this issue Jul 4, 2018 · 1 comment
Closed

Not resolved instanceof property #4

eapunk opened this issue Jul 4, 2018 · 1 comment

Comments

@eapunk
Copy link

eapunk commented Jul 4, 2018

I am trying to write rules for such code

<?php
class SomeClass
{
    function method1()
    {
        $query = "update linked_docs set deleted=1 where id='" . $_POST['signed_id'] . "'";
        $this->db->query($query);
    }

    function method2()
    {
        global $focus;
        $focusId = $_REQUEST['record'];
        // $focusId = empty($focus->id) ? $_REQUEST['record'] : $focus->id;
        $where = "notes.parent_id='{$focusId}' AND notes.filename IS NOT NULL";
        $focus->get_full_list($where);
    }
}

I know classes of these objects but obviously analyzer fails to find class names.

Your example with 'ESAPI->validator' doesn't work for me.

I'd prefer to consider such code as vulnerabilities when function name is in sinks rules and class name is not resolved. Like this

diff --git a/package/src/progpilot/Inputs/MyInputs.php b/package/src/progpilot/Inputs/MyInputs.php
-                    if (!is_null($myclass) && $mysink->get_instanceof_name() === $myclass->get_name()) {
+                    if (empty($myclass) || $mysink->get_instanceof_name() === $myclass->get_name()) {
                         return $mysink;
-                                if ($myclass->get_name() === $mysink_instance_name) {
+                                if (empty($myclass) || $myclass->get_name() === $mysink_instance_name) {
                                     return $mysink;
eric-therond added a commit that referenced this issue Jul 8, 2018
@eric-therond
Copy link
Collaborator

I think your proposition is a good idea, but I am not sure about the possible impacts on false positives rate, maybe this could be configurated (if exactClassName (proposed name) is set to true so the behaviour is the same as today and if it's set to false the behaviour is like you proposed) and ideally this parameter must impact not only the sinks but also sources, validators and sanitizers).

For your initial problem I have fixed it and now you can use this sink :

{"name": "query", "instanceof": "SomeClass->db", "parameters": [{"id": 1, "condition": "QUOTES"}], "language": "php", "attack": "sql_injection", "cwe": "CWE_89"},

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants