designsecurity / progpilot

A static analysis tool for security
MIT License
331 stars 61 forks source link

Add support for defining static method calls as sources #54

Closed ivansaurio closed 1 year ago

ivansaurio commented 1 year ago

Prestashop uses Tools::getValue() to get user input from $_GET and $_POST

I've been trying to define that method as a source with no sucess

Test code:

        $c = Tools::getValue();
        include($c);

I've tried this config and some variations:

{"sources": [
    {"name": "getValue", "is_function": true, "language": "php"}
]}

getSourceByName finds my definition and executes the first return https://github.com/designsecurity/progpilot/blob/master/package/src/progpilot/Inputs/MyInputsInternalApi.php#L290

eric-therond commented 1 year ago

Hello with last version on master it's now possible to define return of a static method as a source: https://github.com/designsecurity/progpilot/blob/master/package/src/uptodate_data/php/dev/sources.json#L9

ivansaurio commented 1 year ago

Hi Eric, That's working as expected, thanks

Now I'm trying to setup sinks. Prestashop provides a few ways to access the database, but the most common one seems to be Db::getInstance()

$db = Db::getInstance(_PS_USE_SQL_SLAVE_);
$db->executeS($sql); //Raw query
$db->getRow($sql); //Raw query, return first row

I wasn't able to add those to the config file. Is that possible?

I was able to fake this by using a helper file:

<?php

class Db {
    public static function getInstance() {
        return new Instance();
    }
}

class Instance {
    public function executeS($query) {

    }
}
{
    "sinks": [
        {"name": "executeS", "instanceof": "Instance", "language": "php", "attack": "sql_injection", "cwe": "CWE_89"}
    ]
}
eric-therond commented 1 year ago

The trick is to define a "custom rule": https://github.com/designsecurity/progpilot/blob/master/docs/CUSTOM_ANALYSIS.md#create-an-object

        {
            "name": "getInstance", 
            "instanceof": "Db", 
            "description": "Instance prestashop db",
            "language": "php", 
            "action": "DEFINE_OBJECT", 
            "extra": "PrestashopDb"
        }

This rule will create an object type with a name of your choice, above it's "PrestashopDb" when the getInstance method of the Db class type is called. Then you can use the "PrestashopDb" type in your sinks:

        {"name": "executeS", "instanceof": "PrestashopDb", "language": "php", "attack": "sql_injection", "cwe": "CWE_89"}

And this vulnerability will be triggered:

<?php
$sql = $_GET["p"];

$db = Db::getInstance(_PS_USE_SQL_SLAVE_);
$db->executeS($sql); //Raw query
$db->getRow($sql); //Raw query, return first row
ivansaurio commented 1 year ago

Thanks eric, that works