✅ Add ReDos check test #81
Conversation
| def test_redos_attack_header(self): | ||
| t, p = parse_options_header(b'application/x-www-form-urlencoded; !="\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\') | ||
| # If vulnerable, this test wouldn't finish, the line above would hang | ||
| self.assertIn(b'"\\', p[b'!']) |
There was a problem hiding this comment.
@Kludex @tiangolo Failure could be made more explicit using a context manager TimeOut similar to this: https://stackoverflow.com/questions/59994077/pytest-assert-for-timeout
There was a problem hiding this comment.
The problem is precisely that there's no easy way to timeout a RegEx, not in async, not in a thread, the only way is to put it on another process and forcefully kill the process... that's also the nature of the attack, and that's also why using a RegEx, at least that particular RegEx was unfeasible. It's because while the RegEx is being processed, that's happening on the CPython side, so there are no standard event loop mechanics at place, not an async event loop and not even the infinite parse/execute loop from CPython (as I understand it).
Not sure if there's a way to improve this, but at least this checks that if CI is passing, this is not happening. Nevertheless, if you find a way to do it without the extra cost of starting a new process, please let me know! I would be super curious about it. 🤓
There was a problem hiding this comment.
Oh OK! No I don't know about magic here... I thought/assumed the encapsulation in a thread would work out! I guess the problem would be the same using re2 ?
One possible way, but maybe an overkill, would be to run that test on PyPy, with the possibility to override the class with a decorator, but that's also not soooo clean.
I spent some time looking for possible (testing) solutions in cpython itself and could not find a good approach. But I've found this bug which I wonder if you found it too: https://bugs.python.org/issue39503
Which led to this PR: https://github.com/python/cpython/pull/19305/files
And looking at the actual urllib2 implementation, I think it is still subject to ReDOS attacks like the one you pointed, isn't it?
✅ Add ReDos check test
Tests that a vulnerability like Starlette's https://github.com/encode/starlette/security/advisories/GHSA-93gm-qmq6-w238 and FastAPI's https://github.com/tiangolo/fastapi/security/advisories/GHSA-qf9m-vfgh-m389, that were really vulnerabilities in python-multipart, don't happen again.