juice-shop
Fix XEE challenge solved on Linux systems because of wrongly detected Windows keyword
Signed-off-by: Stephan Pillhofer [email protected]
Description
Fixes a logical bug inside the XEE challenge which causes the challenge to be solved because of a wrongly detected Windows keyword on a Linux machine (in my case Ubuntu 20 on WSL2), namely "display".
The keyword "display" is detected in the following line of the psswd file:
gdm:x:121:129:Gnome Display Manager:/var/lib/gdm3:/bin/false
As a solution the keyword was removed. Moreover, a ?? operator was replaced with the || operator because if the check for a Windows keyword evaluates to false, the check for a potential Linux keyword is not carried out at all and false is returned immediately.
Resolved or fixed issue: none
Affirmation
- [x] My code follows the CONTRIBUTING.md guidelines
I noticed that some pipelines fail because of another mistake in the code which is not related to the changes in this pull request. Do they need to be resolved in order for this PR to be accaptable?
thx :)
I am a bit lost on the problem that is being addressed here. If you are on Linux, retrieve the passwd file and by coincidence the XXE checks positive with a match on the word 'display' in that file, you still had a successful XXE. If anything, the Windows file check is a bit vague all in itself, but that single keyword is not an actual issue. If you had a user "boot" on Linux you'd end up with the same mismatch.
The change from ?? to || seems reasonable, but as I'm normally not using ??, I'll have to check why I did here. Maybe it was just a linting recommendation, but I'm not sure.
I'll investigate and let you know what I dig up... 😅
Yes you changed it to '??' because of a linting issue as far as I can see in the commit messages. :)
I will try to explain the problem again, sorry if I was not clear about it.
Yes, I am on a Linux machine, so the check if the xml file contains the content of the psswd file should be the reason why I solved the challenge. However, the check if I am on a Windows machine is carried out first. This check finds the keyword 'display'. This leads to the challenge being solved because the server thinks I successfully retrieved the Windows file, even though I am on Linux and actually retrieved the Linux file. However the check for the linux file is not carried out as the Windows check already succeeded. I just noticed this wrong behaviour and it confused me and thats what I wanted to address in that quick PR. :)
Yes, the challenge is still solvable of course, but only because of the wrong detection of one of those Windows keywords. If the keywords weren't falsly detected, the challange actually will not be possible to solve for Linux users because of the ?? operator.
But you are right that removing just the one keyword isn't a proper solution. I see that now. I think, as you implied, a better way for windows detection is needed, in combination with using || instead of ??.
Based on wikipedia, (https://en.wikipedia.org/wiki/SYSTEM.INI) it should be enough to just search for the string "; for 16-bit app support" to identify this windows file, as this comment should be included in all system.ini files. I also checked a few forums which explicitly confirm that this string should be part of Win7 and Win10 System.ini files. (To indicate backwards compatibility) This string has a very low chance of existing in the passwd file. What do you think? I can change the PR accordingly.
Agreed, let's reduce the check to only that one ; for 16-bit app support line. ~Should be good to go then. And replacing the ?? with || also seems fine.~ (Already done)
This PR got accidentally closed by me (because of a force push?). Created a new one, see above ...