Skip to content

Commit 5f8ea39

Browse files
committed
JS: Do not flag auth endpoints that are immune to Login CSRF
1 parent 66b1612 commit 5f8ea39

File tree

1 file changed

+21
-1
lines changed

1 file changed

+21
-1
lines changed

javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ predicate isCsrfProtectionRouteHandler(Routing::RouteHandler handler) {
129129
handler = getAHandlerSettingCsrfCookie()
130130
}
131131

132+
/** Gets a call to `passport.authenticate` */
133+
API::CallNode passportAuthenticateCall() {
134+
result = API::moduleImport("passport").getMember("authenticate").getACall()
135+
}
136+
132137
/**
133138
* A call of form `passport.authenticate(..., { session: false })`, implying that the incoming
134139
* request must carry its credentials rather than relying on cookies.
@@ -137,10 +142,23 @@ predicate isCsrfProtectionRouteHandler(Routing::RouteHandler handler) {
137142
* reduce noise we do not want to flag them.
138143
*/
139144
API::CallNode nonSessionBasedAuthMiddleware() {
140-
result = API::moduleImport("passport").getMember("authenticate").getACall() and
145+
result = passportAuthenticateCall() and
141146
result.getParameter(1).getMember("session").getARhs().mayHaveBooleanValue(false)
142147
}
143148

149+
/**
150+
* Gets a middleware node that performs authentication and is considered immune to Login CSRF.
151+
*
152+
* These are not considered CSRF protectors, but shouldn't themselves be flagged as being vulnerable
153+
* to CSRF. In particular, if the middleware is wrapped by a router handler function, we want to avoid
154+
* spuriously reporting the wrapper function.
155+
*/
156+
API::CallNode authMiddlewareImmuneToCsrf() {
157+
result = passportAuthenticateCall() and
158+
// The local strategy does not provide its own CSRF protection
159+
not result.getArgument(0).getStringValue() = "local"
160+
}
161+
144162
/**
145163
* Gets an express route handler expression that is either a custom CSRF protection middleware,
146164
* or a CSRF protecting library.
@@ -179,6 +197,8 @@ where
179197
not hasCsrfMiddleware(handler) and
180198
// Sometimes the CSRF protection comes later in the same route setup.
181199
not setup.getAChild*() = getACsrfMiddleware() and
200+
// Ignore auth routes that are immune to Login CSRF
201+
not handler.getAChild*() = Routing::getNode(authMiddlewareImmuneToCsrf()) and
182202
// Only warn for dangerous handlers, such as for POST and PUT.
183203
setup.getOwnHttpMethod().isUnsafe()
184204
select cookie, "This cookie middleware is serving a request handler $@ without CSRF protection.",

0 commit comments

Comments
 (0)