theowni
commented
4 months ago Hi @HannoZ, thanks for submitting your solution!
I found 2 issues in your solution:
SSRF:
In your code you're validating URL suffix to make sure that only image can be downloaded. This could be potentially bypassed by providing a URL with extra parameters which will pass your validation but endpoint will be reached and password reseted e.g. the following URL will bypass your logic:
http://localhost:8080/reset-chef-password?bypass=test.png.
Also, it's a good practice to specify only allowed domains for such URLs. As a more secure solution, take a look at @GangGreenTemperTatum solution described here.
RCE
Instead of shell=True, I'd recommend to use shell=False in get_disk_usage and pass arguments as a list. In this way, you will execute only one binary and pass arguments to it. It will be even more secure than a validation library. Such libraries are great help but sometimes they can be bypassed and in this case disabling shell option is more robust.
Think about these recommendations and let me know when you apply the fixes. In general, you did a great job! 👏👏
btw. thank you for the feedback! I will think about implementing some hints mechanism in CLI.
HannoZ
Adding myself to the hall of fame after fixing the vulnerabilities :) An overview of the fixes can be found here
One suggestion that could make the challenges more fun is to move the hints to a separate location (eg. hints.md that you can check if you get stuck). Now for most challenges it is immediately clear where the fix needs to be applied