riboseinc / retrace

retrace is a versatile security vulnerability / bug discovery tool through monitoring and modifying the behavior of compiled binaries on Linux, OpenBSD/FreeBSD/NetBSD (shared object) and macOS (dynamic library).
Other
60 stars 19 forks source link

Check for possible relative paths in system() #290

Closed pablo-mendoza closed 7 years ago

pablo-mendoza commented 7 years ago

Implements this: https://github.com/riboseinc/retrace/issues/163

I figured that even when it may not catch 100% of the cases (since system() basically dumps the command into the shell and that has a lot of escaping, expansions and such) we might as well implement the most common ones and word the warning as "MAY contain a relative path".

ronaldtse commented 7 years ago

@erikbor your comments please 👍

pablo-mendoza commented 7 years ago

I would take a guess and think that the majority of the cases are a single command and that's it. So the simplest check is checking if the command starts with a / or not.

The next more common case I guess are multiple commands (check a / after a semicolon) and pipes (check for / after |)

Again taking a wild guess I'm thinking that should covert the vast majority ofl the cases you might see in the wild AND the code seems very simple to me.

Attempting to cover 100% of the cases would be pretty much impossible, since it depends on what shell the user is running and that can be anything. But I think what I have is a good example of the 80/20 rule. Do 20% of the work, cover 80% of the cases.

erikbor commented 7 years ago

@riataman nice one Pablo! Yeah the 80/20 applies. I don't think there are very complicated system() constructions out there in the wild. Most of them are a single command or several chained together with a semicolon.