hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-03 04:39:29 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #4054 from erik-krogh/urlIncludes

Browse Source 
Approved by esbena
This commit is contained in:
CodeQL CI
2020-08-17 13:54:25 +01:00
committed by GitHub
parent 8876dd51c7 15a74493e0
commit c917cd02bd
4 changed files with 14 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
change-notes/1.26/analysis-javascript.md
View File

@@ -24,7 +24,7 @@
| **Query** | **Expected impact** | **Change** |
|--------------------------------|------------------------------|---------------------------------------------------------------------------|
| Incomplete URL substring sanitization (`js/incomplete-url-substring-sanitization`) | More results | This query now recognizes additional URLs when the substring check is an inclusion check. |
## Changes to libraries

4
javascript/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.ql
View File

@@ -43,6 +43,10 @@ where
or
// target is a HTTP URL to a domain on any TLD
target.regexpMatch("(?i)https?://([a-z0-9-]+\\.)+([a-z]+)(:[0-9]+)?/?")
or
// target is a HTTP URL to a domain on any TLD with path elements, and the check is an includes check
check instanceof StringOps::Includes and
target.regexpMatch("(?i)https?://([a-z0-9-]+\\.)+([a-z]+)(:[0-9]+)?/[a-z0-9/_-]+")
) and
(
if check instanceof StringOps::StartsWith

3
javascript/ql/test/query-tests/Security/CWE-020/IncompleteUrlSubstringSanitization.expected
View File

@@ -20,3 +20,6 @@
| tst-IncompleteUrlSubstringSanitization.js:63:4:63:33 | x.index ... !== -1 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:63:14:63:25 | "secure.com" | secure.com |
| tst-IncompleteUrlSubstringSanitization.js:64:3:64:26 | x.inclu ... e.com") | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:64:14:64:25 | "secure.com" | secure.com |
| tst-IncompleteUrlSubstringSanitization.js:66:6:66:29 | x.inclu ... e.com") | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:66:17:66:28 | "secure.com" | secure.com |
| tst-IncompleteUrlSubstringSanitization.js:73:5:73:48 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:73:15:73:42 | "https: ... oo/bar" | https://secure.com/foo/bar |
| tst-IncompleteUrlSubstringSanitization.js:74:5:74:40 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:74:15:74:34 | "https://secure.com" | https://secure.com |
| tst-IncompleteUrlSubstringSanitization.js:75:5:75:52 | x.index ... ") >= 0 | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.js:75:15:75:46 | "https: ... ar-baz" | https://secure.com/foo/bar-baz |

7
javascript/ql/test/query-tests/Security/CWE-020/tst-IncompleteUrlSubstringSanitization.js
View File

@@ -67,5 +67,10 @@
} else {
doSomeThingWithTrustedURL(x);
}
}
x.startsWith("https://secure.com/foo/bar"); // OK - a forward slash after the domain makes prefix checks safe.
x.indexOf("https://secure.com/foo/bar") >= 0 // NOT OK - the url can be anywhere in the string.
x.indexOf("https://secure.com") >= 0 // NOT OK
x.indexOf("https://secure.com/foo/bar-baz") >= 0 // NOT OK - the url can be anywhere in the string.
});