Skip to content

Commit d0e94e6

Browse files
committed
JS: Exclude error handling from auth calls
1 parent 400bf10 commit d0e94e6

File tree

2 files changed

+7
-2
lines changed

2 files changed

+7
-2
lines changed

javascript/ql/lib/semmle/javascript/security/SensitiveActions.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ class AuthorizationCall extends SensitiveAction, DataFlow::CallNode {
147147
AuthorizationCall() {
148148
exists(string s | s = this.getCalleeName() |
149149
// name contains `login` or `auth`, but not as part of `loginfo` or `unauth`;
150-
// also exclude `author`
151-
s.regexpMatch("(?i).*(login(?!fo)|(?<!un)auth(?!or\\b)|verify).*") and
150+
// also exclude `author` and words followed by `err` (as in `error`)
151+
s.regexpMatch("(?i).*(login(?!fo)|(?<!un)auth(?!or\\b)|verify)(?!err).*") and
152152
// but it does not start with `get` or `set`
153153
not s.regexpMatch("(?i)(get|set).*")
154154
)

javascript/ql/test/query-tests/Security/CWE-770/tst.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,8 @@ express().get('/:path', catchAsync(expensiveHandler1)); // NOT OK
7777
express().get('/:path', rateLimiterMiddleware, catchAsync(expensiveHandler1)); // OK
7878
express().get('/:path', catchAsync(rateLimiterMiddleware), expensiveHandler1); // OK
7979
express().get('/:path', catchAsync(rateLimiterMiddleware), catchAsync(expensiveHandler1)); // OK
80+
81+
function errorHandler(req, res, next) {
82+
next(makeOAuthError(req, res));
83+
}
84+
express().use(errorHandler); // OK - does not perform authentication

0 commit comments

Comments
 (0)