8000 Eliminate false positives by detecting non-stream objects returned fr… · github/codeql@26f0aeb · GitHub
[go: up one dir, main page]
Skip to content

Commit 26f0aeb

Browse files
NapalysNapalys
committed
Eliminate false positives by detecting non-stream objects returned from pipe() calls based on accessed properties
1 parent b6b64d2 commit 26f0aeb

File tree

3 files changed

+36
-3
lines changed

3 files changed

+36
-3
lines changed

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

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ string getNonchainableStreamMethodName() {
4848
result = ["read", "write", "end", "pipe", "unshift", "push", "isPaused", "wrap", "emit"]
4949
}
5050

51+
/**
52+
* Gets the property names commonly found on Node.js streams.
53+
*/
54+
string getStreamPropertyName() {
55+
result =
56+
[
57+
"readable", "writable", "destroyed", "closed", "readableHighWaterMark", "readableLength",
58+
"readableObjectMode", "readableEncoding", "readableFlowing", "readableEnded", "flowing",
59+
"writableHighWaterMark", "writableLength", "writableObjectMode", "writableFinished",
60+
"writableCorked", "writableEnded", "defaultEncoding", "allowHalfOpen", "objectMode",
61+
"errored", "pending", "autoDestroy", "encoding", "path", "fd", "bytesRead", "bytesWritten",
62+
"_readableState", "_writableState"
63+
]
64+
}
65+
5166
/**
5267
* Gets all method names commonly found on Node.js streams.
5368
*/
@@ -109,6 +124,25 @@ predicate isPipeFollowedByNonStreamMethod(PipeCall pipeCall) {
109124
)
110125
}
111126

127+
/**
128+
* Holds if the pipe call result is used to access a property that is not typical of streams.
129+
*/
130+
predicate isPipeFollowedByNonStreamProperty(PipeCall pipeCall) {
131+
exists(DataFlow::PropRef propRef |
132+
propRef = pipeResultRef(pipeCall).getAPropertyRead() and
133+
not propRef.getPropertyName() = getStreamPropertyName()
134+
)
135+
}
136+
137+
/**
138+
* Holds if the pipe call result is used in a non-stream-like way,
139+
* either by calling non-stream methods or accessing non-stream properties.
140+
*/
141+
predicate isPipeFollowedByNonStreamAccess(PipeCall pipeCall) {
142+
isPipeFollowedByNonStreamMethod(pipeCall) or
143+
isPipeFollowedByNonStreamProperty(pipeCall)
144+
}
145+
112146
/**
113147
* Gets a reference to a stream that may be the source of the given pipe call.
114148
* Uses type back-tracking to trace stream references in the data flow.
@@ -145,6 +179,6 @@ predicate hasErrorHandlerRegistered(PipeCall pipeCall) {
145179
from PipeCall pipeCall
146180
where
147181
not hasErrorHandlerRegistered(pipeCall) and
148-
not isPipeFollowedByNonStreamMethod(pipeCall)
182+
not isPipeFollowedByNonStreamAccess(pipeCall)
149183
select pipeCall,
150184
"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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,5 @@
1111
| test.js:143:5:143:62 | stream. ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1212
| test.js:171:5:171:50 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1313
| test.js:175:5:175:39 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
14-
| test.js:179:17:179:40 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1514
| test.js:183:17:183:40 | notStre ... itable) | Stream pipe without error handling on the source stream. Errors won't propagate downstream and may be silently dropped. |
1615
| test.js:193:5:193:32 | copyStr ... nation) | 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ function test() {
176176
}
177177
{ // Member access on a non-stream after pipe
178178
const notStream = getNotAStream();
179-
const val = notStream.pipe(writable).someMember; // $SPURIOUS:Alert
179+
const val = notStream.pipe(writable).someMember;
180180
}
181181
{ // Member access on a stream after pipe
182182
const notStream = getNotAStream();

0 commit comments

Comments
 (0)
0