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

JS: RegExp unknown flags support and enhanced compatibility with RegExp objects #18089

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
178da21
JS: Added test case for CWE-178 RegExp with unknown flags
Napalys Nov 25, 2024
e38b63e
JS: previously js/case-sensitive-middleware-path was not taking into …
Napalys Nov 25, 2024
d6372ae
Update javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql
Napalys Nov 25, 2024
41f21d4
JS: Added test case which is not flagged but should be abusing new Re…
Napalys Nov 26, 2024
38be0e4
JS: Now BadHtmlSanitizers also flags new RegExp as potential issue
Napalys Nov 26, 2024
89f3b6f
JS: Added test case for bad sanitizer with unknown flags, currently n…
Napalys Nov 26, 2024
18c7b18
JS: Now BadHtmlSanitizers new RegExp with unknown flags is also flagged.
Napalys Nov 26, 2024
41fef0f
JS: Added test cases which cover new RegExp creation with replace on …
Napalys Nov 26, 2024
faef9dd
JS: protyte poluting now treats unknownFlags as potentially good sani…
Napalys Nov 26, 2024
7db6f7c
JS: Added test cases with new RegExp for Tainted paths, currently wor…
Napalys Nov 26, 2024
eca7a88
JS: Fixed docs description
Napalys Nov 26, 2024
23b18ae
JS: Now unknown flags are not flagged in taint paths
Napalys Nov 26, 2024
155f1fc
JS: Added test cases for unsafe shell command sanitization with RegEx…
Napalys Nov 26, 2024
a0df33c
JS: UnsafeShellCommand Using unknown flags in the RegExp object is no…
Napalys Nov 26, 2024
aa557cf
JS: Added tests for DotRemovingReplaceCall with RegExp Object.
Napalys Nov 27, 2024
875478c
JS: Fixed path query not flagging new RegExp with DotRemovingReplaceCall
Napalys Nov 27, 2024
9c2366a
JS: Added tests for ReDos with unknownFlags, everything seems to be good
Napalys Nov 27, 2024
7631803
JS: Add test cases for RegExp object usage in replace within incomple…
Napalys Nov 27, 2024
1ae1748
JS: incomplete sanitization now also works with RegExp objects
Napalys Nov 27, 2024
98fd977
JS: imcomplete sanization now handles properly maybe global
Napalys Nov 27, 2024
fe28657
JS: add test cases with unknown flags for double escaping, works as e…
Napalys Nov 27, 2024
dbae553
JS: add xss test cases with unknownflags for replace using RegExp
Napalys Nov 27, 2024
c71778f
JS: xss does not flag anymore replace with RegExp unknown flags
Napalys Nov 27, 2024
1ca57cf
JS: add test cases with RegExp object for MaskingReplacer, currently …
Napalys Nov 27, 2024
a2c4674
JS: fixed issue where MaskingReplacer would work only with regexp lit…
Napalys Nov 27, 2024
e673348
JS: now RegExp with unknown flags is not flagged as an issue within p…
Napalys Nov 27, 2024
62194f5
JS: add test cases RegExp with unknown flags
Napalys Nov 27, 2024
1d2e08a
JS: now Reg Exp injection treats unknownFlags as sanitization, Metach…
Napalys Nov 27, 2024
9a1c1f4
JS: Added in RegExpCreationNode maybeGlobal predicate for more conven…
Napalys Nov 27, 2024
fd77360
Added change notes
Napalys Nov 28, 2024
9ca0fe4
Update RegExp handling and add test case
Napalys Nov 28, 2024
d2de9a2
Fixed change notes
Napalys Nov 28, 2024
13afd63
Update javascript/ql/lib/change-notes/2024-11-28-regexp-unknown-flags.md
Napalys Nov 29, 2024
3171f38
JS: fixed bad alert messages when it came to incomplete sanitization …
Napalys Nov 29, 2024
9d4e737
JS: follow proper code standards for get predicates
Napalys Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions javascript/ql/src/Security/CWE-116/IncompleteSanitization.ql
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,26 @@ predicate whitelistedRemoval(StringReplaceCall repl) {
)
}

from StringReplaceCall repl, DataFlow::Node old, string msg
/**
* Gets a nice string representation of the pattern or value of the node.
*/
predicate getPatternOrValueString(DataFlow::Node node, string patternOrValue) {
asgerf marked this conversation as resolved.
Show resolved Hide resolved
if node instanceof DataFlow::RegExpConstructorInvokeNode
then
exists(DataFlow::RegExpConstructorInvokeNode regExp |
node = regExp and
patternOrValue = "/" + regExp.getRoot() + "/"
)
asgerf marked this conversation as resolved.
Show resolved Hide resolved
else patternOrValue = node.toString()
}

from StringReplaceCall repl, DataFlow::Node old, string patternOrValue, string msg
where
(old = repl.getArgument(0) or old = repl.getRegExp()) and
getPatternOrValueString(old, patternOrValue) and
(
not repl.maybeGlobal() and
msg = "This replaces only the first occurrence of " + old + "." and
msg = "This replaces only the first occurrence of " + patternOrValue + "." and
asgerf marked this conversation as resolved.
Show resolved Hide resolved
// only flag if this is likely to be a sanitizer or URL encoder or decoder
exists(string m | m = getAMatchedString(old) |
// sanitizer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
| tst.js:149:2:149:24 | x.repla ... replace | This replaces only the first occurrence of "\\n". |
| tst.js:193:9:193:17 | s.replace | This replaces only the first occurrence of /'/. |
| tst.js:202:10:202:18 | p.replace | This replaces only the first occurrence of "/../". |
| tst.js:341:9:341:17 | p.replace | This replaces only the first occurrence of new Reg ... .\\\\./"). |
| tst.js:341:9:341:17 | p.replace | This replaces only the first occurrence of /\\.\\.//. |
| tst.js:345:9:345:17 | s.replace | This does not escape backslash characters in the input. |
| tst.js:349:9:349:17 | s.replace | This replaces only the first occurrence of new RegExp("\\'"). |
| tst.js:349:9:349:17 | s.replace | This replaces only the first occurrence of /'/. |
| tst.js:353:9:353:17 | s.replace | This does not escape backslash characters in the input. |
| tst.js:362:2:362:10 | x.replace | This replaces only the first occurrence of new RegExp("\\n"). |
| tst.js:363:2:363:24 | x.repla ... replace | This replaces only the first occurrence of new RegExp("\\n"). |
| tst.js:362:2:362:10 | x.replace | This replaces only the first occurrence of /\n/. |
| tst.js:363:2:363:24 | x.repla ... replace | This replaces only the first occurrence of /\n/. |