SonarQube 显示正则表达式拒绝服务 (ReDoS)
我在 JavaScript 中使用正则表达式验证日期,但是当我运行 SonarQube 进行代码分析时.它将正则表达式显示为安全漏洞.
示例 1:
下面是正则表达式模式(链接到正则表达式的来源https://stackoverflow.com/a/15504877/13353721):
^(?:(?:31(/|-|.)(?:0?[13578]|1[02]))1|(?:(?:29|30)(/|-|.)(?:0?[13-9]|1[0-2])2))(?:(?:1[6-9]|[2-9]d)?d{2})$|^(?:29(/|-|.)0?23(?:(?:(?:1[6-9]|[2-9]d)?(?:0[48]|[2468][048]|[13579][26])|(?:(?:16|[2468][048]|[3579][26])00))))$|^(?:0?[1-9]|1d|2[0-8])(/|-|.)(?:(?:0?[1-9])|(?:1[0-2]))4(?:(?:1[6-9]|[2-9]d)?d{2})$
示例 2:
对于浮动值,我使用了下面的正则表达式
^d{1,5}(?:.d{1,5})?$
SonarQube 引发了同样的安全错误,我尝试了各种不同的正则表达式模式,但都不起作用.
解决方案热点与漏洞
首先请注意,SonarQube 通知您的是安全热点,而不是漏洞.这意味着(引自 文档):p><块引用>
安全热点突出显示开发人员需要审查的安全敏感代码段.经过审查,您会发现没有威胁,或者您需要应用修复程序来保护代码.
[...]
使用热点,会突出显示对安全敏感的一段代码,但整体应用程序的安全性可能不会受到影响.由开发人员检查代码以确定是否需要修复来保护代码.
这里重要的一点是,SonarQube 并没有告诉您有什么问题.它告诉您应该仔细查看代码以确定是否有问题.
换句话说,它告诉您您的正则表达式可能容易受到 ReDoS 攻击,但实际上并非如此.如果您查看代码并确定不存在漏洞,则无需更改任何内容即可忽略该问题.
那么,SonarQube 究竟为什么要告诉您检查此代码?
SonarQube 实际上并不检测正则表达式是否容易受到 ReDoS 攻击(这就是为什么它被标记为安全热点,而不是漏洞).相反,它会标记所有重要的正则表达式,并提醒您检查它们以确定它们是否易受攻击.正如规则文档中所解释的,它认为任何正则表达式都是不平凡的包含多次出现的任何字符 *+{
.
由于您的两个正则表达式都符合该标准,因此都被标记.
更新:以上内容适用于编写此答案时的 ReDoS 规则.同时,该规则已被弃用,取而代之的是一条新规则,该规则只应抱怨实际上具有超线性运行时的正则表达式.新规则不会抱怨这个问题中的正则表达式.
那么您的代码是否容易受到攻击?
不,您的正则表达式都不容易受到攻击.事实上,在任一表达式中使用的唯一重复运算符是 {}
,由于您在所有情况下都提供了上限,因此甚至没有任何无限重复.
但是,我想说您的第一个正则表达式非常复杂,足以成为可读性和维护方面的噩梦.所以你应该考虑用另一种方法替换它(例如将字符串拆分为单独的数字并检查每个数字是否在所需的范围内).
那你该怎么办?
确定正则表达式不易受到攻击后,您应该关闭热点.
在评论中指出,如果您将正则表达式字符串拆分为多个连接字符串或将其移动到变量中,该消息将消失.起作用的原因很简单,就是它欺骗 SonarQube 找不到正则表达式.因此,这种更改不会使您的代码变得更好或更安全,只会使 SonarQube 感到困惑,并且绝不比仅仅关闭消息更可取.通常不建议为了让您的静态分析工具闭嘴而混淆您的代码.
I am validating date using regex in JavaScript but when I run SonarQube for code analysis. It is showing the regex as a security vulnerability.
Example 1:
Below is the regex pattern (link to source of regex https://stackoverflow.com/a/15504877/13353721):
^(?:(?:31(/|-|.)(?:0?[13578]|1[02]))1|(?:(?:29|30)(/|-|.)(?:0?[13-9]|1[0-2])2))(?:(?:1[6-9]|[2-9]d)?d{2})$|^(?:29(/|-|.)0?23(?:(?:(?:1[6-9]|[2-9]d)?(?:0[48]|[2468][048]|[13579][26])|(?:(?:16|[2468][048]|[3579][26])00))))$|^(?:0?[1-9]|1d|2[0-8])(/|-|.)(?:(?:0?[1-9])|(?:1[0-2]))4(?:(?:1[6-9]|[2-9]d)?d{2})$
Example 2:
For floating value I have used the below regex
^d{1,5}(?:.d{1,5})?$
SonarQube is throwing the same security error, I tried various different regex patterns, but it is not working.
解决方案Hotspot vs. Vulnerability
First of all note that SonarQube is informing you about a security hotspot, not a vulnerability. What that means is (quoting from the docs):
A Security Hotspot highlights a security-sensitive piece of code that the developer needs to review. Upon review, you'll either find there is no threat or you need to apply a fix to secure the code.
[...]
With a Hotspot, a security-sensitive piece of code is highlighted, but the overall application security may not be impacted. It's up to the developer to review the code to determine whether or not a fix is needed to secure the code.
The important takeaway here is that SonarQube is not telling you that there is something wrong. It's telling you that you should look carefully at the code to determine whether something is wrong.
In other words it's telling you that your regex may be vulnerable to ReDoS attacks, not that it actually is. If you review the code and determine that there is no vulnerability, it is perfectly fine to just dismiss the issue without changing anything.
So why exactly is SonarQube telling you to review this code?
SonarQube doesn't actually detect whether a regular expression is vulnerable to ReDoS or not (that's why it's labelled as a security hotspot, not a vulnerability). Instead it flags all non-trivial regular expressions and reminds you to review them to determine whether they're vulnerable or not. As explained in the documentation of the rule, it considers any regex as non-trivial that contains more than one occurrence of any of the characters *+{
.
Since both of your regular expressions are non-trivial by that criteria, both are flagged.
Update: The above applies to the ReDoS rule that was around when this answer was written. This rule has been deprecated in the mean time and replaced with a new rule that should only complain about regular expressions that actually have a superlinear runtime. The new rule does not complain about the regex in this question.
So is your code vulnerable?
No, neither of your regular expressions are vulnerable. In fact the only repetition operator used in either expression is {}
and since you provide an upper bound in all cases, there isn't even any unbounded repetition.
However, I'd say your first regex is complicated enough to be a readability and maintenance nightmare. So you should consider replacing it with another approach (such as splitting the string into individual numbers and checking that each number is in the desired range).
So what should you do?
Having determined that the regular expressions are not vulnerable, you should dismiss the hotspot.
In the comments it was pointed out, that the message will go away if you split the regex string into multiple concatenated strings or move it into a variable. The reason that works is simply that it tricks SonarQube into not finding the regex. So that change wouldn't make your code any better or safer, it would just confuse SonarQube and is in no way preferable to just dismissing the message. It is generally not advisable to obfuscate your code just to make your static analysis tools shut up.
相关文章