fnando / i18n

A small library to provide the I18n translations on the JavaScript.
https://fnando.github.io/i18n/
MIT License
169 stars 20 forks source link

Possible CWE-185 Incorrect Regular Expression #77

Closed khalilgharbaoui closed 1 year ago

khalilgharbaoui commented 1 year ago

https://github.com/fnando/i18n/blob/27e14e7b7f25150261b4d05b27d4ea1ca83256b7/src/helpers/interpolate.ts#L47-L49

Incorrect Regular Expression Severity: Medium RegExp() called with a variable, this might allow an attacker to DOS your application with a long-running regular expression.

fnando commented 1 year ago

Hi!

CWE-185 doesn't seem to apply here (it specifies wrong validation and allow bypassing these validations). On top of that, the regex is not applied on user-input strings; instead, it's applied on whatever it was defined by the developer as the source string.

I saw #78 and I'm curious. Could you please explain how breaking the replacement in a separate variable assignment would fix the issue?

khalilgharbaoui commented 1 year ago

Hi @fnando,

The reason i wrote "Possible" is because my SAST pipeline came up with this vulnerability and it might be a false positive. The file (public/javascripts/i18n.js) which includes the interpolate function ends up in the rails public folder after exporting strings using i18n js which raised some concerns. TBH i'm not sure if the breakup solves the issue but it silences the SAST scanner. But the file which ends up in the public folder is overwritten each time i run the export, can't seem to get it to persist without editing the package it self. Also I'm still on V3 but even if i would upgrade to V4 this specific piece of code is still identical.

Any ideas on how to resolve this are welcome.

Thanks

fnando commented 1 year ago

what are you using to scan vulnerabilities? I can try it and see what's up.

khalilgharbaoui commented 1 year ago

I'm basically using semgrep doing:

semgrep scan ./public --config=auto
fnando commented 1 year ago

just checked, and this is def a false positive. My suggestion is to exclude the file using something like this.