Gallopsled / pwntools

CTF framework and exploit development library
http://pwntools.com
Other
11.99k stars 1.7k forks source link

add "retguard" property and display it with checksec #2297

Closed jasperla closed 9 months ago

jasperla commented 11 months ago

quoting http://undeadly.org/cgi?action=article;sid=20180606064444, retguard is an "anti-ROP security mechanism, which uses random per-function cookies to protect return addresses on the stack."

Changelog

After creating your Pull Request, please add and push a commit that updates the changelog for the appropriate branch.
You can look at the existing changelog for examples of how to do this.

peace-maker commented 11 months ago

Thank you, but I'm not sure this is useful to show the status by default. It seems this is an OpenBSD only feature and showing "No retguard found" for every binary seems spammy. Maybe just show it if the ELf is for OpenBSD and the retguard is implemented for the architecture? Correct me if there are Linux retguard binaries around.

https://isopenbsdsecu.re/mitigations/retguard/

peace-maker commented 9 months ago

Looking at proper OS detection of ELF files seems like a rabbit hole. Right now we just statically assume "Linux" with the occational exception for "Android".

The file command looks at the NT_GNU_VERSION note since the e_ident.ei_osabi ELF header field is often set to 0 -> SYSV even if it's not sysv. There are other indicators too like the name of the dynamic interpreter (which we already use for Android detection, which might be improved by the android note).

So something like this could work for only displaying retguard status for openbsd

    @property
    def openbsd(self):
        """:class:`bool`: Whether this ELF is an OpenBSD binary"""
        for note in self.iter_notes():
            if note.n_type == 'NT_GNU_ABI_TAG' and note.n_name == 'OpenBSD':
                return True
        return False

capa has some more evolved OS heuristics to give some context on the possible complexity.

Nonetheless, this seems out of scope for the purpose of this PR. I think it's acceptable to merge the above openbsd check into the retguard property in this PR. Making the retguard a ternary return between [None, False, True] returning None if the ELF doesn't target OpenBSD. And only display the retguard status in checksec if it isn't None.

peace-maker commented 9 months ago

After further discussion we came to the conclusion that implementing openbsd detection to avoid output of the retguard status on any other ELFs not targeting openbsd would require proper context.os openbsd support. Hacking the check into ELF just for openbsd is bad since it's shadowing ELF.os and adding openbsd to ELF.os requires more work around the code base to allow os extraction when using context.binary to set the context.

Thank you for your contribution but we'd rather not add partial openbsd support here now. This pull request can be used for reference and reopened once proper OS detection is implemented. Please leave a comment if you disagree or see another solution! If anyone wants to use pwntools to target openbsd hosts, you're welcome to join the Discord to discuss possible patches.