From bb3e3a541d69a6f01abb1cea906a67270abb05ce Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 14 Dec 2018 10:24:30 +0100 Subject: [PATCH] JS: address doc review comments --- change-notes/1.20/analysis-javascript.md | 2 +- .../ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp | 5 ++++- .../ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/change-notes/1.20/analysis-javascript.md b/change-notes/1.20/analysis-javascript.md index 17930ffc192..d268c3a39d0 100644 --- a/change-notes/1.20/analysis-javascript.md +++ b/change-notes/1.20/analysis-javascript.md @@ -11,7 +11,7 @@ | **Query** | **Tags** | **Purpose** | |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | Double escaping or unescaping (`js/double-escaping`) | correctness, security, external/cwe/cwe-116 | Highlights potential double escaping or unescaping of special characters, indicating a possible violation of [CWE-116](https://cwe.mitre.org/data/definitions/116.html). Results are shown on LGTM by default. | -| Incomplete regular expression for hostnames (`js/incomplete-hostname-regexp`) | correctness, security, external/cwe/cwe-020 | Highlights hostname sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default.| +| Incomplete regular expression for hostnames (`js/incomplete-hostname-regexp`) | correctness, security, external/cwe/cwe-020 | Highlights hostname sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default.| | Incomplete URL substring sanitization | correctness, security, external/cwe/cwe-020 | Highlights URL sanitizers that are likely to be incomplete, indicating a violation of [CWE-020](https://cwe.mitre.org/data/definitions/20.html). Results shown on LGTM by default. | | Incorrect suffix check (`js/incorrect-suffix-check`) | correctness, security, external/cwe/cwe-020 | Highlights error-prone suffix checks based on `indexOf`, indicating a potential violation of [CWE-20](https://cwe.mitre.org/data/definitions/20.html). Results are shown on LGTM by default. | | Useless comparison test (`js/useless-comparison-test`) | correctness | Highlights code that is unreachable due to a numeric comparison that is always true or always false. Results are shown on LGTM by default. | diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp index 0b2b7369bc1..08756cf9cd5 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp +++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.qhelp @@ -21,7 +21,7 @@ Even if the check is not used in a security-critical context, the incomplete check may still cause undesirable behaviors - when the check succeeds accidentally. + when it accidentally succeeds.

@@ -55,6 +55,9 @@ example.com, effectively allowing the redirect to go to an attacker-controlled domain such as wwwXexample.com. +

+

+ Address this vulnerability by escaping . appropriately: let regex = /(www|beta|)\.example\.com/. diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql index abe6d76d6f0..01335fbbe0a 100644 --- a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql +++ b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql @@ -1,6 +1,6 @@ /** * @name Incomplete regular expression for hostnames - * @description Matching a URL or hostname against a regular expression that contains an unescaped dot as part of the hostname might match more than expected. + * @description Matching a URL or hostname against a regular expression that contains an unescaped dot as part of the hostname might match more hostnames than expected. * @kind problem * @problem.severity warning * @precision high