0xdea / semgrep-rules

A collection of my Semgrep rules to facilitate vulnerability research.
https://semgrep.dev
MIT License
541 stars 54 forks source link

raptor-ret-stack-address not checking for `static` variables #2

Open parsiya opened 2 years ago

parsiya commented 2 years ago

The raptor-ret-stack-address rule checks if we are returning a pointer to a stack variable.

It returns a false positive if we are returning a static variable.

static SomeObject* getObject()
{
    static SomeObject o;
    return &o;
}

This usually happens inside a static function.

Adding all the edge cases will be a pain, we basically have to add a pattern-not-inside for each pattern-inside.

I tried filtering the $TYPE metavariable and filtering with metavariable-regex. But $TYPE only contains the type (e.g., SomeObject here) and not static.

You could also decide that the false positives are worth keeping the rule simple which is definitely a good tradeoff.

0xdea commented 2 years ago

This is indeed a known limitation in my rules (shared also by the raptor-write-into-stack-buffer rule for instance). Based on my tests, Semgrep does not seem to support static variable declarations and other modifiers such as register/volatile (at least not consistently). I've been meaning to open an issue with Semgrep, but I haven't had the time to better investigate this behaviour yet.

There are some other examples of (apparent) lack of support by Semgrep:

This is probably due to the fact that C/C++ support is still experimental.

Thanks again for your interest! Let me know if you find a clever way to overcome these limitations.

parsiya commented 2 years ago

Thanks for the reply. C/C++ support is indeed still experimental but more folks are asking for support and it might get on the roadmap, soon.

I am gonna try and see if I can manually include static in pattern-not-inside and see if it works. It worked with my limited test case.

Again, thanks for the great rules.