Commit Graph

9352 Commits

Author SHA1 Message Date
Asger F
1df69ec1d2 JS: Actually don't propagate into array element 0
Preserving tainted-url-suffix into array element 0 seemed like a good idea, but didn't work out so well.
2024-09-12 13:42:36 +02:00
Asger F
0e4e0f4fdd JS: Preverse tainted-url-suffix when stepping into prefix
A URL of form https://example.com?evil#bar will contain '?evil' after splitting out the '#' suffix, and vice versa.
2024-09-12 13:42:28 +02:00
Asger F
74ab346348 JS: Do not include taint steps in TaintedUrlSuffix::step
TaintedUrlSuffix is currently only used in TaintTracking configs meaning it is already propagated
by taint steps. The inclusion of these taint steps here however meant that implicit reads could appear prior to any of these steps.

This was is problematic for PropRead steps as an expression like x[0] could spuriously read from array element 1 via the path:

x [element 1]
x [empty access path] (after implicit read)
x[0] (taint step through PropRead)
2024-09-12 13:42:25 +02:00
Asger F
2712bf821a JS: Fix a bug in isSafeClientSideUrlProperty 2024-09-12 13:42:23 +02:00
Asger F
bc04131c72 JS: Disallow implicit reads before an optional step 2024-09-12 13:42:22 +02:00
Asger F
e1bed42481 JS: Add inline expectation test specifically for TaintedUrlSuffix 2024-09-12 13:42:20 +02:00
Asger F
cf90c83604 JS: Accept changes to nodes/edges results 2024-09-12 13:42:19 +02:00
Asger F
3b09bc548e JS: Add taint step for shift() 2024-09-12 13:42:17 +02:00
Asger F
3ea1134cc1 JS: Add inline test for .shift() method 2024-09-12 13:42:16 +02:00
Asger F
3fcf4ef7a1 JS: More precise model of .shift()
Array.prototype.shift only returns the first array element.

The mutation of Argument[this] is not yet modelled, and is better handled when we have use-use flow.
2024-09-12 13:42:15 +02:00
Asger F
e4f7560bcd JS: Add missing qldoc 2024-09-12 13:42:14 +02:00
Asger F
15fc450a9e JS: Add reminder to update ClientSideUrlRedirect 2024-09-12 13:42:13 +02:00
Asger F
da696817a3 JS: Convert 'split' taint step to legacy taint step 2024-09-12 13:42:05 +02:00
Asger F
133b016c7c JS: Remove old 'split' handling from TaintedUrlSuffix 2024-09-12 13:41:56 +02:00
Asger F
e87e543850 JS: Ensure optional steps/barriers are computed in the correct stage 2024-09-12 13:35:38 +02:00
Asger F
7790f68fe2 JS: Make the TaintedUrlSuffix library use optional steps/barriers 2024-09-12 13:35:36 +02:00
Asger F
3b34cd72f2 JS: Handle split() with '#' or '?' separator in a separate summary
This summary uses the notion of optional steps/barriers so it becomes configurable whether there is flow into the zero'th array element.

Also makes sure we handle the second-argument version of split().
2024-09-12 13:35:33 +02:00
Asger F
24983a5836 JS: Add OptionalStep and OptionalBarrier MaD tokens
OptionalStep[foo] and OptionalBarrier[foo] contribute steps/barriers that are not active by default, but can be opted into by specific queries or for specific flow states.

(Will be used in the following commits)
2024-09-12 13:30:39 +02:00
Asger F
87454a4f11 JS: Remove unused predicate 2024-09-10 14:44:49 +02:00
Asger F
0ddb1c87f5 JS: Test update indicating a problem with .split() 2024-09-10 13:14:37 +02:00
Asger F
e0ca1b0482 JS: Benign test updates 2024-09-10 13:07:24 +02:00
Asger F
3d4287b7cc JS: Remove ContentSet#asArrayIndex()
For ContentSet it is ambiguous whether asArrayIndex() should get a singleton content set, or the KnownArrayElement content set. The user will now have to choose between asSingleton().asArrayIndex() or ContentSet::arrayElementKnown.
2024-09-09 13:28:32 +02:00
Asger F
013d226ae3 JS: Update comment 2024-09-09 13:26:27 +02:00
Asger F
55d4e7e742 JS: Use ArrayElementKnown when reading a constant array index 2024-09-09 13:26:25 +02:00
Asger F
094112c905 Merge pull request #17213 from asgerf/jss/spread-argument
JS: Improve handling of spread arguments and rest parameters [shared data flow branch]
2024-09-09 13:15:22 +02:00
Asger F
fb9732a33f JS: Add another test and TODO about an issue with constant array indices 2024-09-06 08:43:11 +02:00
Asger F
1da68aac73 JS: Benign test output change
This happened as a result of the bugfix in the previous commit
2024-09-06 08:43:10 +02:00
Asger F
a9a8351cce JS: Fix one case of missing handling of unknown array index 2024-09-06 08:43:09 +02:00
Asger F
379c7ef20a JS: Add test to show lack of unknown array element being propagated 2024-09-06 08:43:08 +02:00
Asger F
92bb4b3da8 JS: Address some comments from hvitved 2024-09-05 11:32:07 +02:00
Asger F
4568967a76 JS: Do not use legacy taint steps in TaintedUrlSuffix
Tainted URL suffix steps are added as configuration-specific additional
steps, which means implicit reads may occur before any of these steps.

These steps accidentally included the legacy taint steps which include
a step from 'arguments' to all positional parameters. Combined with the
implicit read, arguments could escape their array index and flow to
any parameter while in the tainted-url flow state.
2024-08-29 13:48:30 +02:00
Asger F
65a36b0b3b JS: Add regression test for argument position confusion 2024-08-29 13:42:28 +02:00
Asger F
f65879eef1 JS: Update a test that no longer fails 2024-08-27 11:35:37 +02:00
Asger F
cb5dbb919d JS: Update test to reflect implicit read flow has been fixed
Shows the effect of https://github.com/github/codeql/pull/17262
2024-08-27 11:35:36 +02:00
Asger F
a2d53c261b JS: Update test output and add related TODO in model of 'async' 2024-08-27 11:35:35 +02:00
Asger F
837a8be1b8 JS: Update test output and add related TODO in 'markdown-table' model 2024-08-27 11:35:34 +02:00
Asger F
2e2181be2c JS: Update test output that only affects nodes/edges/subpaths 2024-08-27 11:35:33 +02:00
Asger F
3e196f83f1 JS: Update Promises/flow2 test 2024-08-27 11:35:32 +02:00
Asger F
aa8bd332bf JS: Add a few more tests 2024-08-27 11:35:31 +02:00
Asger F
371f7ef551 JS: Add implicit taint read of array elements 2024-08-27 11:35:31 +02:00
Asger F
df42e7c527 JS: Add test showing lack of implicit reads for ArrayElement 2024-08-27 11:35:30 +02:00
Asger F
4e7bd9ddd8 JS: Update Arrays test now that array elements do not taint the whole array 2024-08-27 11:35:29 +02:00
Asger F
4389b5c999 JS: Fix issue for .apply() calls 2024-08-27 11:35:28 +02:00
Asger F
34e6864fa3 JS: Note issue with .apply() calls 2024-08-27 11:35:27 +02:00
Asger F
ac1dd1850e JS: Remove taint step from array element to whole array 2024-08-27 11:35:26 +02:00
Asger F
5084d0260f Update tests.expected
The 'arguments' node is only materialised for functions that use 'arguments
2024-08-27 11:35:25 +02:00
Asger F
895cb872ad JS: Add taint into dynamic argument array 2024-08-27 11:35:24 +02:00
Asger F
079a622cf9 JS: Add tests showing missing taint flow
When the spread argument itself is tained and not
inside any content, the read steps currently fail
to propagate the data.
2024-08-27 11:35:23 +02:00
Asger F
6a083136d7 JS: Hide some nodes 2024-08-27 11:35:22 +02:00
Asger F
acdc896c04 JS: Support for dynamic args to flow summaries 2024-08-27 11:35:21 +02:00