hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-16 16:53:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Merge pull request #3672 from esbena/js/server-crashing-route-handler"

Browse Source 
This reverts commit 243e3ad9e3, reversing
changes made to df79f2adc5.
This commit is contained in:
Esben Sparre Andreasen
2020-06-22 14:45:35 +02:00
parent eca5e2df8a
commit 0a8d15ccc4
5 changed files with 0 additions and 202 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
javascript/ql/src/Security/CWE-730/ServerCrash.qhelp
View File

@@ -1,22 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
</overview>
<recommendation>
</recommendation>
<example>
</example>
<references>
</references>
</qhelp>

100
javascript/ql/src/Security/CWE-730/ServerCrash.ql
View File

@@ -1,100 +0,0 @@
/**
* @name Server crash
* @description A server that can be forced to crash may be vulnerable to denial-of-service
* attacks.
* @kind problem
* @problem.severity error
* @precision high
* @id js/server-crash
* @tags security
* external/cwe/cwe-730
*/
import javascript
/**
* Gets a function that `caller` invokes.
*/
Function getACallee(Function caller) {
exists(DataFlow::InvokeNode invk |
invk.getEnclosingFunction() = caller and result = invk.getACallee()
)
}
/**
* Gets a function that `caller` invokes, excluding calls guarded in `try`-blocks.
*/
Function getAnUnguardedCallee(Function caller) {
exists(DataFlow::InvokeNode invk |
invk.getEnclosingFunction() = caller and
result = invk.getACallee() and
not exists(invk.asExpr().getEnclosingStmt().getEnclosingTryCatchStmt())
)
}
predicate isHeaderValue(HTTP::ExplicitHeaderDefinition def, DataFlow::Node node) {
def.definesExplicitly(_, node.asExpr())
}
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "Configuration" }
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node node) {
// using control characters in a header value will cause an exception
isHeaderValue(_, node)
}
}
predicate isLikelyToThrow(DataFlow::Node crash) {
exists(Configuration cfg, DataFlow::Node sink | cfg.hasFlow(_, sink) | isHeaderValue(crash, sink))
}
/**
* A call that looks like it is asynchronous.
*/
class AsyncCall extends DataFlow::CallNode {
DataFlow::FunctionNode callback;
AsyncCall() {
callback.flowsTo(getLastArgument()) and
callback.getParameter(0).getName() = ["e", "err", "error"] and
callback.getNumParameter() = 2 and
not exists(callback.getAReturn())
}
DataFlow::FunctionNode getCallback() { result = callback }
}
/**
* Gets a function that is invoked by `asyncCallback` without any try-block wrapping, `asyncCallback` is in turn is called indirectly by `routeHandler`.
*
* If the result throws an excection, the server of `routeHandler` will crash.
*/
Function getAPotentialServerCrasher(
HTTP::RouteHandler routeHandler, DataFlow::FunctionNode asyncCallback
) {
exists(AsyncCall asyncCall |
// the route handler transitively calls an async function
asyncCall.getEnclosingFunction() =
getACallee*(routeHandler.(DataFlow::FunctionNode).getFunction()) and
asyncCallback = asyncCall.getCallback() and
// the async function transitively calls a function that may throw an exception out of the the async function
result = getAnUnguardedCallee*(asyncCallback.getFunction())
)
}
/**
* Gets an AST node that is likely to throw an uncaught exception in `fun`.
*/
ExprOrStmt getALikelyExceptionThrower(Function fun) {
result.getContainer() = fun and
not exists([result.(Expr).getEnclosingStmt(), result.(Stmt)].getEnclosingTryCatchStmt()) and
(isLikelyToThrow(result.(Expr).flow()) or result instanceof ThrowStmt)
}
from HTTP::RouteHandler routeHandler, DataFlow::FunctionNode asyncCallback, ExprOrStmt crasher
where crasher = getALikelyExceptionThrower(getAPotentialServerCrasher(routeHandler, asyncCallback))
select crasher, "When an exception is thrown here and later exits $@, the server of $@ will crash.",
asyncCallback, "this asynchronous callback", routeHandler, "this route handler"

6
javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.expected
View File

@@ -1,6 +0,0 @@
| server-crash.js:7:5:7:14 | throw err; | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:6:28:8:3 | (err, x ... OK\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |
| server-crash.js:11:3:11:11 | throw 42; | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:50:28:52:3 | (err, x ... ();\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |
| server-crash.js:16:7:16:16 | throw err; | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:15:30:17:5 | (err, x ... K\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |
| server-crash.js:28:5:28:14 | throw err; | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:27:28:29:3 | (err, x ... OK\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |
| server-crash.js:33:5:33:14 | throw err; | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:32:28:34:3 | (err, x ... OK\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |
| server-crash.js:41:5:41:48 | res.set ... header) | When an exception is thrown here and later exits $@, the server of $@ will crash. | server-crash.js:40:28:42:3 | (err, x ... OK\\n } | this asynchronous callback | server-crash.js:31:25:73:1 | (req, r ... });\\n} | this route handler |

1
javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.qlref
View File

@@ -1 +0,0 @@
Security/CWE-730/ServerCrash.ql

73
javascript/ql/test/query-tests/Security/CWE-730/server-crash.js
View File

@@ -1,73 +0,0 @@
const express = require("express");
const app = express();
const fs = require("fs");
function indirection1() {
fs.readFile("/WHATEVER", (err, x) => {
throw err; // NOT OK
});
}
function indirection2() {
throw 42; // NOT OK
}
function indirection3() {
try {
fs.readFile("/WHATEVER", (err, x) => {
throw err; // NOT OK
});
} catch (e) {}
}
function indirection4() {
throw 42; // OK: guarded caller
}
function indirection5() {
indirection6();
}
function indirection6() {
fs.readFile("/WHATEVER", (err, x) => {
throw err; // NOT OK
});
}
app.get("/async-throw", (req, res) => {
fs.readFile("/WHATEVER", (err, x) => {
throw err; // NOT OK
});
fs.readFile("/WHATEVER", (err, x) => {
try {
throw err; // OK: guarded throw
} catch (e) {}
});
fs.readFile("/WHATEVER", (err, x) => {
res.setHeader("reflected", req.query.header); // NOT OK
});
fs.readFile("/WHATEVER", (err, x) => {
try {
res.setHeader("reflected", req.query.header); // OK: guarded call
} catch (e) {}
});
indirection1();
fs.readFile("/WHATEVER", (err, x) => {
indirection2();
});
indirection3();
try {
indirection4();
} catch (e) {}
indirection5();
fs.readFile("/WHATEVER", (err, x) => {
req.query.foo; // OK
});
fs.readFile("/WHATEVER", (err, x) => {
req.query.foo.toString(); // OK
});
fs.readFile("/WHATEVER", (err, x) => {
req.query.foo.bar; // NOT OK [INCONSISTENCY]: need to add property reads as sinks
});
fs.readFile("/WHATEVER", (err, x) => {
res.setHeader("reflected", unknown); // OK
});
});