Skip to content

Commit c27157f

Browse files
NapalysNapalys
committed
Add UnhandledStreamPipee Quality query and tests to detect missing error handlers in Node.js streams
1 parent d1e769b commit c27157f

File tree

4 files changed

+246
-0
lines changed

4 files changed

+246
-0
lines changed

‎javascript/ql/src/Quality/UnhandledStreamPipe.ql‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* @id js/nodejs-stream-pipe-without-error-handling
3+
* @name Node.js stream pipe without error handling
4+
* @description Calling `pipe()` on a stream without error handling may silently drop errors and prevent proper propagation.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @precision high
8+
* @tags quality
9+
* frameworks/nodejs
10+
*/
11+
12+
import javascript
13+
14+
/**
15+
* A call to the `pipe` method on a Node.js stream.
16+
*/
17+
class PipeCall extends DataFlow::MethodCallNode {
18+
PipeCall() { this.getMethodName() = "pipe" }
19+
20+
/** Gets the source stream (receiver of the pipe call). */
21+
DataFlow::Node getSourceStream() { result = this.getReceiver() }
22+
23+
/** Gets the destination stream (argument of the pipe call). */
24+
DataFlow::Node getDestinationStream() { result = this.getArgument(0) }
25+
}
26+
27+
/**
28+
* Gets the method names used to register event handlers on Node.js streams.
29+
* These methods are used to attach handlers for events like `error`.
30+
*/
31+
string getEventHandlerMethodName() { result = ["on", "once", "addListener"] }
32+
33+
/**
34+
* A call to register an event handler on a Node.js stream.
35+
* This includes methods like `on`, `once`, and `addListener`.
36+
*/
37+
class StreamEventRegistration extends DataFlow::MethodCallNode {
38+
StreamEventRegistration() { this.getMethodName() = getEventHandlerMethodName() }
39+
40+
/** Gets the stream (receiver of the event handler). */
41+
DataFlow::Node getStream() { result = this.getReceiver() }
42+
}
43+
44+
/**
45+
* Models flow relationships between streams and related operations.
46+
* Connects destination streams to their corresponding pipe call nodes.
47+
* Connects streams to their event handler registration nodes.
48+
*/
49+
predicate streamFlowStep(DataFlow::Node streamNode, DataFlow::Node relatedNode) {
50+
exists(PipeCall pipe |
51+
streamNode = pipe.getDestinationStream() and
52+
relatedNode = pipe
53+
)
54+
or
55+
exists(StreamEventRegistration handler |
56+
streamNode = handler.getStream() and
57+
relatedNode = handler
58+
)
59+
}
60+
61+
/**
62+
* Gets a reference to a stream that may be the source of the given pipe call.
63+
* Uses type back-tracking to trace stream references in the data flow.
64+
*/
65+
private DataFlow::SourceNode streamRef(DataFlow::TypeBackTracker t, PipeCall pipeCall) {
66+
t.start() and
67+
result = pipeCall.getSourceStream().getALocalSource()
68+
or
69+
exists(DataFlow::SourceNode prev |
70+
prev = streamRef(t.continue(), pipeCall) and
71+
streamFlowStep(result.getALocalUse(), prev)
72+
)
73+
or
74+
exists(DataFlow::TypeBackTracker t2 | result = streamRef(t2, pipeCall).backtrack(t2, t))
75+
}
76+
77+
/**
78+
* Gets a reference to a stream that may be the source of the given pipe call.
79+
*/
80+
private DataFlow::SourceNode streamRef(PipeCall pipeCall) {
81+
result = streamRef(DataFlow::TypeBackTracker::end(), pipeCall)
82+
}
83+
84+
/**
85+
* Holds if the source stream of the given pipe call has an `error` handler registered.
86+
*/
87+
predicate hasErrorHandlerRegistered(PipeCall pipeCall) {
88+
exists(StreamEventRegistration handler |
89+
handler = streamRef(pipeCall).getAMethodCall(getEventHandlerMethodName()) and
90+
handler.getArgument(0).getStringValue() = "error"
91+
)
92+
}
93+
94+
from PipeCall pipeCall
95+
where not hasErrorHandlerRegistered(pipeCall)
96+
select pipeCall,
97+
"Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped."

‎javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
| test.js:4:5:4:28 | stream. ... nation) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
2+
| test.js:19:5:19:17 | s2.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
3+
| test.js:45:5:45:30 | stream2 ... ation2) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
4+
| test.js:60:5:60:30 | stream2 ... ation2) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
5+
| test.js:66:5:66:21 | stream.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
6+
| test.js:79:5:79:25 | s2.pipe ... ation2) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
7+
| test.js:94:5:94:21 | stream.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
8+
| test.js:109:26:109:37 | s.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
9+
| test.js:116:5:116:21 | stream.pipe(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
10+
| test.js:125:5:125:26 | getStre ... e(dest) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |

‎javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.js‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
function test() {
2+
{
3+
const stream = getStream();
4+
stream.pipe(destination); // $Alert
5+
}
6+
{
7+
const stream = getStream();
8+
stream.pipe(destination);
9+
stream.on('error', handleError);
10+
}
11+
{
12+
const stream = getStream();
13+
stream.on('error', handleError);
14+
stream.pipe(destination);
15+
}
16+
{
17+
const stream = getStream();
18+
const s2 = stream;
19+
s2.pipe(dest); // $Alert
20+
}
21+
{
22+
const stream = getStream();
23+
stream.on('error', handleError);
24+
const s2 = stream;
25+
s2.pipe(dest);
26+
}
27+
{
28+
const stream = getStream();
29+
const s2 = stream;
30+
s2.on('error', handleError);
31+
s2.pipe(dest);
32+
}
33+
{
34+
const s = getStream().on('error', handler);
35+
const d = getDest();
36+
s.pipe(d);
37+
}
38+
{
39+
getStream().on('error', handler).pipe(dest);
40+
}
41+
{
42+
const stream = getStream();
43+
stream.on('error', handleError);
44+
const stream2 = stream.pipe(destination);
45+
stream2.pipe(destination2); // $Alert
46+
}
47+
{
48+
const stream = getStream();
49+
stream.on('error', handleError);
50+
const destination = getDest();
51+
destination.on('error', handleError);
52+
const stream2 = stream.pipe(destination);
53+
const s3 = stream2;
54+
s = s3.pipe(destination2);
55+
}
56+
{
57+
const stream = getStream();
58+
stream.on('error', handleError);
59+
const stream2 = stream.pipe(destination);
60+
stream2.pipe(destination2); // $Alert
61+
}
62+
{ // Error handler on destination instead of source
63+
const stream = getStream();
64+
const dest = getDest();
65+
dest.on('error', handler);
66+
stream.pipe(dest); // $Alert
67+
}
68+
{ // Multiple aliases, error handler on one
69+
const stream = getStream();
70+
const alias1 = stream;
71+
const alias2 = alias1;
72+
alias2.on('error', handleError);
73+
alias1.pipe(dest);
74+
}
75+
{ // Multiple pipes, handler after first pipe
76+
const stream = getStream();
77+
const s2 = stream.pipe(destination1);
78+
stream.on('error', handleError);
79+
s2.pipe(destination2); // $Alert
80+
}
81+
{ // Handler registered via .once
82+
const stream = getStream();
83+
stream.once('error', handleError);
84+
stream.pipe(dest);
85+
}
86+
{ // Handler registered with arrow function
87+
const stream = getStream();
88+
stream.on('error', (err) => handleError(err));
89+
stream.pipe(dest);
90+
}
91+
{ // Handler registered for unrelated event
92+
const stream = getStream();
93+
stream.on('close', handleClose);
94+
stream.pipe(dest); // $Alert
95+
}
96+
{ // Error handler registered after pipe, but before error
97+
const stream = getStream();
98+
stream.pipe(dest);
99+
setTimeout(() => stream.on('error', handleError), 8000); // $MISSING:Alert
100+
}
101+
{ // Pipe in a function, error handler outside
102+
const stream = getStream();
103+
function doPipe(s) { s.pipe(dest); }
104+
stream.on('error', handleError);
105+
doPipe(stream);
106+
}
107+
{ // Pipe in a function, error handler not set
108+
const stream = getStream();
109+
function doPipe(s) { s.pipe(dest); } // $Alert
110+
doPipe(stream);
111+
}
112+
{ // Dynamic event assignment
113+
const stream = getStream();
114+
const event = 'error';
115+
stream.on(event, handleError);
116+
stream.pipe(dest); // $SPURIOUS:Alert
117+
}
118+
{ // Handler assigned via variable property
119+
const stream = getStream();
120+
const handler = handleError;
121+
stream.on('error', handler);
122+
stream.pipe(dest);
123+
}
124+
{ // Pipe with no intermediate variable, no error handler
125+
getStream().pipe(dest); // $Alert
126+
}
127+
{ // Handler set via .addListener synonym
128+
const stream = getStream();
129+
stream.addListener('error', handleError);
130+
stream.pipe(dest);
131+
}
132+
{ // Handler set via .once after .pipe
133+
const stream = getStream();
134+
stream.pipe(dest);
135+
stream.once('error', handleError);
136+
}
137+
}

‎javascript/ql/test/query-tests/Quality/UnhandledStreamPipe/test.qlref‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
query: Quality/UnhandledStreamPipe.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)