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.75k stars 1.56k forks source link

False positive - Log injection is not mitigated via replace with Regex argument in Kotlin #17423

Open fercarcedo opened 2 months ago

fercarcedo commented 2 months ago

Description of the false positive

CodeQL is reporting a log injection vulnerability even though I am deleting the problematic characters with Kotlin's replace function call with a Regex as its first parameter.

Reading the query (https://github.com/github/codeql/blob/main/java/ql/lib/semmle/code/java/security/LogInjection.qll) I suspect that's because it searches for uses of replace with either Strings or Chars as arguments (in order to check for line break removal), but not uses of replace with Regex as its first argument (in Kotlin, there is no replaceAll function, there is only a replace that can accept either String, Char or Regex).

I have also looked at the tests (https://github.com/github/codeql/blob/main/java/ql/test/query-tests/security/CWE-117/LogInjectionTest.java) and that's why I belive this might be the reason, as the tests always use replaceAll when working with regular expressions (as it is a Java file).

Code samples or links to source code

     private fun baseSanitize(param: String) = param.replace(Regex("[^a-zA-Z0-9_-]"), "")
     private fun baseSanitize(param: String) = param.replace("[^a-zA-Z0-9_-]".toRegex(), "")
     private fun baseSanitize(param: String) = param.replace("\\W".toRegex(), "")
mbg commented 2 months ago

Hi @fercarcedo šŸ‘‹šŸ»

Thanks for reporting this false positive! Your reasoning makes sense, thanks for looking into that.

Just to let you know, addressing false positives isn't a current product priority, so I can't give you an indication of when this might get addressed, but we will be keeping track of this internally.