Debugger security enhancements
This pull request introduces 2 changes:
- Number of pin-based failed authentication attempts before the debugger is locked (brute force protection) has been reduced from 11 to 10. This was because 11 seemed like an odd number to choose and I think it's likely that the intention was 10 but it is off-by-one.
- Even when the debugger was locked due to too many failed authentication attempts, it was still possible to authenticate using a cookie. This change prevents cookie-based authentication when the debugger is locked for pin-based authentication.
These changes enhance the security of the debugger because it is possible to generate a cookie if knowledge of the environment is known (i.e. APPNAME, CGROUP, FLASKPATH, MACADDRESS, MACHINEID, MODULENAME & SERVICEUSER). It may be possible to access enough information about the environment if the application using werkzeug contains a vulnerability such as arbitrary file read, e.g. using path traversal. This could result in the debugger being used to turn a medium-risk vulnerability into a critical-risk one (i.e. remote code execution).
An exploit module for Metasploit is publicly available that will generate a cookie when provided with the required information (disclaimer: I am the author of the module, but I also want to improve the security of werkzeug, even though it will make the exploit less effective).
One of the checks failed (readthedocs). I don't believe this is under my control and is an error due to a change in the behaviour of readthedocs. I tried to fix this in a different pull request but was unsuccessful.
I suggest that this pull request has the security tag applied.
The development server states everywhere in the docs and terminal that is is only for development and must never be used in production. If a project is exposing the debugger in a non-development situation, then the security report would be to that project. These changes are fine, but are not considered a security fix. The pin is a very mild stopgap in case a project accidentally does expose the debugger, but is not expected to be particularly secure. All those pieces of information used to generate the cookie are used on purpose so that the pin isn't constantly changing.
These changes are fine, but are not considered a security fix.
Thank you for the review and accepting, and for maintaining a great project.
I understand your point about the docs stating not to use this in production, but we all know that there is a non-zero percent chance that it will be used in production.
In fact it was in my role as a penetration tester that I found the debugger enabled behind a PIN/cookie in production and was able to gain access to the required information to generate the PIN and cookie using a separate arbitrary file read in the affected application then use the debugger to gain code execution on the system.
I would argue that the PIN/cookie is a security control, and that this was therefore a security issue.
In any case, I’m glad I could help improve the security of the project and that you helped facilitate that so thank you again.