github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.51k stars 1.49k forks source link

LGTM.com - false positive - Regex handling Path Injection #6436

Open sobeaa opened 3 years ago

sobeaa commented 3 years ago

Description of the false positive

I use regex to handle the Path Injection.

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/Deasilsoft/a2j/snapshot/1257a1ab48aaa9ea4a564dc93ca45e4c0a817c18/files/src/a2j/util.py?sort=name&dir=ASC&mode=heatmap#x20ee52e2c34e65c0:1

yoff commented 3 years ago

Hi @TyrotoxismB, thank you for the report. I agree that this is a false positive. I was initially worried that your solution did not consider directory traversal encodings (such as %2e%2e%2f), but in fact our recommended solution does not deal with this either. I consulted with our security lab and we are going to update our recommendations. As such, I will also recommend you to consider traversal encodings.

Meanwhile, it appears that the query fails both at identifying your normalising regex (in is_record) and your prefix ensurance (in get_record), so I will take a look at updating the query also :-)

sobeaa commented 3 years ago

Thank you for your great work. Thank you for bringing up the topic of traversal encoding which I've seem to forgotten about. If LGTM detects missing traversal encoding my brain capacity can be focused on being happy instead of all this forgetting-business :)

Gosh my jokes are terrible! lmao