Conversation
|
Thanks for your submission @porcupineyhairs, sorry for the late reply, my life has been kinda hectic ⚡ 😴 I agree with the general idea behind these query changes, it will be more accurate if we can track user-controlled data 👍 However, since this query is now promoted to be part of our standard query suite, the code quality will also need to match that of our other queries. So you would need to follow the structure of our other path-problems, which looks like this (using SQL injection as an example):
(as always, ensure proper QLDocs and auto formatting) |
|
@RasmusWL Changes done! PTAL. |
The current PAM auth bypass query which was contributed by me a few months back, alert on a vulenrable function but does not check if the function is actually function. This leads to a lot of fasle positives. With this PR, I add a taint-tracking configuration to check if the username parameter can actually be supplied by an attacker. This should bring the FP's significantly down.
Should not be needed
There was a problem hiding this comment.
Thanks @porcupineyhairs 👍 I took the liberty of fixing up a few other small things, so now it looks good to me 👍
I might get some more 👀 on the select text, but other than that, I don't see anything holding us back from merging this.
Please don't make more force pushes to the PR, so we can retain the history 😊
|
@RasmusWL Sorry about that. The force push was due to a bunch of unrelated work getting commit with this one. By mistake, I had added about 80-90 go files. |
no problem. I just meant from now on. My bad if that was unclear. |
fix typo Co-authored-by: intrigus-lgtm <60750685+intrigus-lgtm@users.noreply.github.com>
The current PAM auth bypass query which was contributed by me a few months back, alert on a vulenrable function but does not check if the function is actually function. This leads to a lot of fasle positives.
With this PR, I add a taint-tracking configuration to check if the username parameter can actually be supplied by an attacker.
This should bring the FP's significantly down.
CC: @RasmusWL