FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

JS: experimental query for a multi-character pattern applied to a single stream chunk by ninadphalak · Pull Request #22738 · github/codeql · GitHub

  1. github
  2. codeql

Repository navigation

JS: experimental query for a multi-character pattern applied to a single stream chunk #22738

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.

Sign up for GitHub

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

Open
ninadphalak wants to merge 1 commit into
base: main
Choose a base branch
Could not load branches
Branch not found: {{ refName }}

Are you sure you want to change the base?

Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
from
Conversation 0 Commits 1 Checks 0 Findings Files changed
Open

JS: experimental query for a multi-character pattern applied to a single stream chunk #22738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .expected  (1) .js  (3) .qhelp  (1) .ql  (1) .qlref  (1) All 5 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
54 changes: 54 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-693/SplitBoundaryChunkFilter.qhelp
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
A stream arrives in chunks whose boundaries are chosen by the sender, the transport or the
tokenizer, not by the receiving code. When a filter applies a regular expression to each chunk
on its own, a value that starts in one chunk and ends in the next is never seen as a whole:
neither piece matches, so a redaction filter forwards the value unchanged and a blocking filter
lets it through. The failure only shows up when a boundary happens to fall inside a match, so
tests that feed the filter whole strings pass.
</p>
<p>
This query flags a regular expression that can match two or more characters when it is applied
to a value derived from the current chunk alone inside a per-chunk callback: a Node.js
<code>Transform</code> or <code>Writable</code> implementation, a web <code>TransformStream</code>
transformer, a <code>'data'</code> event listener, or the body of a <code>for await</code> loop.
Applying the pattern to the chunk joined with carried text (for example a held-back tail or an
accumulated buffer) is not flagged.
</p>
</overview>

<recommendation>
<p>
Carry state between chunks. Hold back a tail at least as long as the longest possible match,
prepend it to the next chunk before matching, emit only the part that can no longer be the start
of a match, and flush the tail when the stream ends. Alternatively buffer to a natural boundary
(a line, a message) and apply the pattern once per boundary. For patterns with unbounded
matches, set an explicit cap and treat reaching it as a reportable condition rather than
silently cutting the match.
</p>
</recommendation>

<example>
<p>
In the following example the pattern is applied to each chunk separately. An email address
that is split across two chunks, such as <code>jane.do</code> and <code>e@example.com</code>, is
forwarded without redaction.
</p>
<sample src="examples/SplitBoundaryChunkFilterBad.js" />
<p>
The corrected version keeps the last 64 characters of the previous chunk, matches against the
join, and releases the held tail at the end of the stream.
</p>
<sample src="examples/SplitBoundaryChunkFilterGood.js" />
</example>

<references>
<li>CWE-693: <a href="https://cwe.mitre.org/data/definitions/693.html">Protection Mechanism Failure</a>.</li>
<li>T. Ptacek and T. Newsham, <a href="https://insecure.org/stf/secnet_ids/secnet_ids.html">Insertion, Evasion, and Denial of Service: Eluding Network Intrusion Detection</a>, 1998: the same mechanism at the network layer.</li>
<li>N. Phalak, <a href="https://doi.org/10.5281/zenodo.22909585">Split-Boundary Leaks in Streaming Guardrails</a>: measurements across streaming redaction filters.</li>
</references>
</qhelp>
151 changes: 151 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-693/SplitBoundaryChunkFilter.ql
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/**
* @name Pattern applied to a single stream chunk without carried state
* @description A regular expression that can match more than one character is applied to one
* chunk of a stream at a time, and nothing from the previous chunk is carried over.
* A value that straddles two chunks is never matched, so a redaction or blocking
* filter forwards it unchanged.
* @kind path-problem
* @problem.severity warning
* @security-severity 5.3
* @precision medium
* @id js/split-boundary-chunk-filter
* @tags security
* experimental
* external/cwe/cwe-693
*/

import javascript

/** A term that matches the empty string only (anchors, boundaries, lookarounds). */
predicate zeroWidth(RegExpTerm t) {
t instanceof RegExpAnchor or
t instanceof RegExpWordBoundary or
t instanceof RegExpNonWordBoundary or
t instanceof RegExpSubPattern
}

/** Holds if `t` can match a string of two or more characters. */
predicate multiChar(RegExpTerm t) {
t instanceof InfiniteRepetitionQuantifier
or
t.(RegExpRange).getUpperBound() > 1
or
t.(RegExpConstant).getValue().length() > 1
or
count(RegExpTerm c | c = t.(RegExpSequence).getAChild() and not zeroWidth(c)) > 1
or
multiChar(t.getAChild())
}

/** The value that holds one chunk inside a per-chunk stream callback. */
DataFlow::Node chunkValue() {
// class method _transform(chunk, ...) / _write(chunk, ...)
exists(MethodDeclaration m | m.getName() = ["_transform", "_write"] |
result = DataFlow::parameterNode(m.getBody().getParameter(0))
)
or
// Mastra processors: processOutputStream({ part, ... })
exists(MethodDeclaration m, PropertyPattern pp |
m.getName() = "processOutputStream" and
pp.getName() = "part" and
pp.getContainer() = m.getBody() and
result = DataFlow::valueNode(pp.getValuePattern().(VarDecl).getVariable().getAnAccess())
)
or
// new Transform({ transform(chunk, ...) }), new TransformStream({ transform(chunk, controller) }),
// new Writable({ write(chunk, ...) }), through2(function (chunk, enc, cb) {...})
exists(DataFlow::InvokeNode inv, DataFlow::FunctionNode f |
inv.getCalleeName() =
["Transform", "TransformStream", "Writable", "Duplex", "through2", "through"] and
(
f =
inv.getAnArgument()
.getALocalSource()
.(DataFlow::ObjectLiteralNode)
.getAPropertyWrite(["transform", "write"])
.getRhs()
.getAFunctionValue()
or
f = inv.getAnArgument().getAFunctionValue()
) and
result = f.getParameter(0)
)
or
// stream.on('data', chunk => ...)
exists(DataFlow::MethodCallNode on |
on.getMethodName() = "on" and
on.getArgument(0).getStringValue() = "data" and
result = on.getArgument(1).getAFunctionValue().getParameter(0)
)
or
// for await (const chunk of stream) { ... }
exists(ForOfStmt fo | fo.isAwait() |
result = DataFlow::valueNode(fo.getLValue().(VarDecl).getVariable().getAnAccess())
)
}

/**
* A string operation that applies the regular expression `re` to the string `str`.
* `resolved` is false when the regular expression object could not be traced to a literal,
* which only happens for `exec` and `test`, whose receiver must be a regular expression.
*/
predicate regexApplication(DataFlow::MethodCallNode mc, DataFlow::Node str, boolean resolved) {
exists(RegExpTerm re | re.isRootTerm() and multiChar(re) and resolved = true |
mc.getMethodName() = ["replace", "replaceAll", "match", "matchAll", "search", "split"] and
re = RegExp::getRegExpFromNode(mc.getArgument(0)) and
str = mc.getReceiver()
or
mc.getMethodName() = ["test", "exec"] and
re = RegExp::getRegExpFromNode(mc.getReceiver()) and
str = mc.getArgument(0)
)
or
mc.getMethodName() = ["test", "exec"] and
not exists(RegExp::getRegExpFromNode(mc.getReceiver())) and
str = mc.getArgument(0) and
resolved = false
}

module ChunkConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source = chunkValue() }

predicate isSink(DataFlow::Node sink) { regexApplication(_, sink, _) }

predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
// the chunk's fields (payload.text, choices[0].delta.content, ...)
succ.(DataFlow::PropRead).getBase() = pred
or
// decoding and trimming keep it the same chunk
exists(DataFlow::MethodCallNode mc | succ = mc and mc.getReceiver() = pred |
mc.getMethodName() =
[
"toString", "trim", "trimStart", "trimEnd", "toLowerCase", "toUpperCase", "normalize",
"valueOf"
]
)
or
exists(DataFlow::CallNode c | succ = c and c.getArgument(0) = pred |
c.getCalleeName() = ["String", "decode", "decodeURIComponent", "from"]
)
}
}

module ChunkFlow = DataFlow::Global<ChunkConfig>;

import ChunkFlow::PathGraph

from
ChunkFlow::PathNode source, ChunkFlow::PathNode sink, DataFlow::MethodCallNode mc,
boolean resolved
where
ChunkFlow::flowPath(source, sink) and
regexApplication(mc, sink.getNode(), resolved)
select mc, source, sink,
"A pattern " +
any(string s |
if resolved = true
then s = "that can match more than one character"
else s = "of unknown width"
) +
" is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched.",
source.getNode(), "a single stream chunk"
9 changes: 9 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-693/examples/SplitBoundaryChunkFilterBad.js
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const { Transform } = require('stream');
const EMAIL = /[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g;

// BAD: each chunk is matched on its own; an address split across two chunks is forwarded as is
const redact = new Transform({
transform(chunk, encoding, callback) {
callback(null, chunk.toString().replace(EMAIL, '[REDACTED]'));
},
});
20 changes: 20 additions & 0 deletions javascript/ql/src/experimental/Security/CWE-693/examples/SplitBoundaryChunkFilterGood.js
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const { Transform } = require('stream');
const EMAIL = /[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g;
const HOLD = 64; // longer than any address the pattern can match

// GOOD: the last HOLD characters are carried into the next chunk and released at the end
class Redact extends Transform {
constructor() {
super();
this.tail = '';
}
_transform(chunk, encoding, callback) {
const joined = this.tail + chunk.toString();
const out = joined.replace(EMAIL, '[REDACTED]');
this.tail = out.slice(-HOLD);
callback(null, out.slice(0, -HOLD));
}
_flush(callback) {
callback(null, this.tail.replace(EMAIL, '[REDACTED]'));
}
}
66 changes: 66 additions & 0 deletions javascript/ql/test/experimental/Security/CWE-693/SplitBoundaryChunkFilter.expected
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
edges
| test.js:8:14:8:18 | chunk | test.js:9:14:9:18 | chunk | provenance | |
| test.js:9:14:9:18 | chunk | test.js:9:14:9:29 | chunk.toString() | provenance | Config |
| test.js:15:13:15:17 | chunk | test.js:16:18:16:22 | chunk | provenance | |
| test.js:16:11:16:14 | text | test.js:17:14:17:17 | text | provenance | |
| test.js:16:18:16:22 | chunk | test.js:16:18:16:39 | chunk.t ... 'utf8') | provenance | Config |
| test.js:16:18:16:39 | chunk.t ... 'utf8') | test.js:16:11:16:14 | text | provenance | |
| test.js:23:13:23:17 | chunk | test.js:24:24:24:28 | chunk | provenance | |
| test.js:31:11:31:15 | delta | test.js:32:11:32:15 | delta | provenance | |
| test.js:31:19:31:23 | chunk | test.js:31:19:31:31 | chunk.choices | provenance | Config |
| test.js:31:19:31:31 | chunk.choices | test.js:31:19:31:34 | chunk.choices[0] | provenance | Config |
| test.js:31:19:31:34 | chunk.choices[0] | test.js:31:19:31:40 | chunk.c ... ].delta | provenance | Config |
| test.js:31:19:31:40 | chunk.c ... ].delta | test.js:31:19:31:48 | chunk.c ... content | provenance | Config |
| test.js:31:19:31:48 | chunk.c ... content | test.js:31:11:31:15 | delta | provenance | |
| test.js:38:19:38:22 | data | test.js:39:22:39:25 | data | provenance | |
| test.js:39:22:39:25 | data | test.js:39:15:39:26 | String(data) | provenance | Config |
| test.js:45:13:45:17 | chunk | test.js:46:51:46:55 | chunk | provenance | |
| test.js:46:51:46:55 | chunk | test.js:46:51:46:66 | chunk.toString() | provenance | Config |
| test.js:54:13:54:17 | chunk | test.js:55:26:55:30 | chunk | provenance | |
| test.js:55:11:55:11 | s | test.js:56:24:56:24 | s | provenance | |
| test.js:55:15:55:49 | dec.dec ... true }) | test.js:55:11:55:11 | s | provenance | |
| test.js:55:26:55:30 | chunk | test.js:55:15:55:49 | dec.dec ... true }) | provenance | Config |
| test.js:62:13:62:17 | chunk | test.js:63:14:63:18 | chunk | provenance | |
| test.js:63:14:63:18 | chunk | test.js:63:14:63:29 | chunk.toString() | provenance | Config |
nodes
| test.js:8:14:8:18 | chunk | semmle.label | chunk |
| test.js:9:14:9:18 | chunk | semmle.label | chunk |
| test.js:9:14:9:29 | chunk.toString() | semmle.label | chunk.toString() |
| test.js:15:13:15:17 | chunk | semmle.label | chunk |
| test.js:16:11:16:14 | text | semmle.label | text |
| test.js:16:18:16:22 | chunk | semmle.label | chunk |
| test.js:16:18:16:39 | chunk.t ... 'utf8') | semmle.label | chunk.t ... 'utf8') |
| test.js:17:14:17:17 | text | semmle.label | text |
| test.js:23:13:23:17 | chunk | semmle.label | chunk |
| test.js:24:24:24:28 | chunk | semmle.label | chunk |
| test.js:31:11:31:15 | delta | semmle.label | delta |
| test.js:31:19:31:23 | chunk | semmle.label | chunk |
| test.js:31:19:31:31 | chunk.choices | semmle.label | chunk.choices |
| test.js:31:19:31:34 | chunk.choices[0] | semmle.label | chunk.choices[0] |
| test.js:31:19:31:40 | chunk.c ... ].delta | semmle.label | chunk.c ... ].delta |
| test.js:31:19:31:48 | chunk.c ... content | semmle.label | chunk.c ... content |
| test.js:32:11:32:15 | delta | semmle.label | delta |
| test.js:38:19:38:22 | data | semmle.label | data |
| test.js:39:15:39:26 | String(data) | semmle.label | String(data) |
| test.js:39:22:39:25 | data | semmle.label | data |
| test.js:45:13:45:17 | chunk | semmle.label | chunk |
| test.js:46:51:46:55 | chunk | semmle.label | chunk |
| test.js:46:51:46:66 | chunk.toString() | semmle.label | chunk.toString() |
| test.js:54:13:54:17 | chunk | semmle.label | chunk |
| test.js:55:11:55:11 | s | semmle.label | s |
| test.js:55:15:55:49 | dec.dec ... true }) | semmle.label | dec.dec ... true }) |
| test.js:55:26:55:30 | chunk | semmle.label | chunk |
| test.js:56:24:56:24 | s | semmle.label | s |
| test.js:62:13:62:17 | chunk | semmle.label | chunk |
| test.js:63:14:63:18 | chunk | semmle.label | chunk |
| test.js:63:14:63:29 | chunk.toString() | semmle.label | chunk.toString() |
subpaths
#select
| test.js:9:14:9:58 | chunk.t ... CTED]') | test.js:8:14:8:18 | chunk | test.js:9:14:9:29 | chunk.toString() | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:8:14:8:18 | chunk | a single stream chunk |
| test.js:17:14:17:60 | text.re ... [SSN]') | test.js:15:13:15:17 | chunk | test.js:17:14:17:17 | text | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:15:13:15:17 | chunk | a single stream chunk |
| test.js:24:24:24:60 | chunk.r ... CTED]') | test.js:23:13:23:17 | chunk | test.js:24:24:24:28 | chunk | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:23:13:23:17 | chunk | a single stream chunk |
| test.js:32:11:32:44 | delta.r ... CTED]') | test.js:31:19:31:23 | chunk | test.js:32:11:32:15 | delta | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:31:19:31:23 | chunk | a single stream chunk |
| test.js:39:15:39:67 | String( ... [KEY]') | test.js:38:19:38:22 | data | test.js:39:15:39:26 | String(data) | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:38:19:38:22 | data | a single stream chunk |
| test.js:46:9:46:67 | /-----B ... ring()) | test.js:45:13:45:17 | chunk | test.js:46:51:46:66 | chunk.toString() | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:45:13:45:17 | chunk | a single stream chunk |
| test.js:56:24:56:53 | s.repla ... CTED]') | test.js:54:13:54:17 | chunk | test.js:56:24:56:24 | s | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:54:13:54:17 | chunk | a single stream chunk |
| test.js:63:14:63:72 | chunk.t ... ), '#') | test.js:62:13:62:17 | chunk | test.js:63:14:63:29 | chunk.toString() | A pattern that can match more than one character is applied to $@ with no state carried from the previous chunk; a value that straddles two chunks is not matched. | test.js:62:13:62:17 | chunk | a single stream chunk |
1 change: 1 addition & 0 deletions javascript/ql/test/experimental/Security/CWE-693/SplitBoundaryChunkFilter.qlref
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/Security/CWE-693/SplitBoundaryChunkFilter.ql
Loading
Back | FazBrowse Home | New Git URL