Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fercarcedo opened this issue Sep 10, 2024 · 1 comment

Comments

@fercarcedo
Copy link

fercarcedo commented Sep 10, 2024

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
Copy link
Member

mbg commented Sep 12, 2024

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.

@mbg mbg added the Kotlin label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants