mirror of
https://github.com/github/codeql.git
synced 2026-03-05 23:26:51 +01:00
add js/session-fixation query
This commit is contained in:
48
javascript/ql/src/Security/CWE-384/SessionFixation.qhelp
Normal file
48
javascript/ql/src/Security/CWE-384/SessionFixation.qhelp
Normal file
@@ -0,0 +1,48 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
Reusing a session could allow an attacker to gain unauthorized access to another account. Always
|
||||
ensure that, when a user logs in or out, the current session is abandoned so that a new
|
||||
session may be started.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
|
||||
<p>
|
||||
Always use <code>req.session.regenerate(...);</code> to start a new session when
|
||||
a user logs in or out.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
|
||||
<p>
|
||||
The following example shows the previous session being used after authentication.
|
||||
This would allow a previous user to use the new user's account.
|
||||
</p>
|
||||
|
||||
<sample src="examples/SessionFixation.js" />
|
||||
|
||||
<p>
|
||||
This code example solves the problem by not reusing the session, and instead calling <code>req.session.regenerate()</code>
|
||||
to ensure that the session is not reused.
|
||||
</p>
|
||||
<sample src="examples/SessionFixationFixed.js" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
<li>
|
||||
OWASP: <a href="https://www.owasp.org/index.php/Session_fixation">Session fixation</a>
|
||||
</li>
|
||||
<li>
|
||||
Stackoverflow: <a href="https://stackoverflow.com/questions/22209354/creating-a-new-session-after-authentication-with-passport/30468384#30468384">Creating a new session after authentication with Passport</a>
|
||||
</li>
|
||||
<li>
|
||||
jscrambler.com: <a href="https://blog.jscrambler.com/best-practices-for-secure-session-management-in-node">Best practices for secure session management in Node</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
58
javascript/ql/src/Security/CWE-384/SessionFixation.ql
Normal file
58
javascript/ql/src/Security/CWE-384/SessionFixation.ql
Normal file
@@ -0,0 +1,58 @@
|
||||
/**
|
||||
* @name Failure to abandon session
|
||||
* @description Reusing an existing session as a different user could allow
|
||||
* an attacker to access someone else's account by using
|
||||
* their session.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 5
|
||||
* @precision medium
|
||||
* @id js/session-fixation
|
||||
* @tags security
|
||||
* external/cwe/cwe-384
|
||||
*/
|
||||
|
||||
import javascript
|
||||
|
||||
/**
|
||||
* Holds if `setup` uses express-session (or similar) to log in a user.
|
||||
*/
|
||||
pragma[inline]
|
||||
predicate isLoginSetup(Express::RouteSetup setup) {
|
||||
// either some path that contains "login" with a write to `req.session`
|
||||
setup.getPath().matches("%login%") and
|
||||
exists(
|
||||
setup
|
||||
.getARouteHandler()
|
||||
.(Express::RouteHandler)
|
||||
.getARequestSource()
|
||||
.ref()
|
||||
.getAPropertyRead("session")
|
||||
.getAPropertyWrite()
|
||||
)
|
||||
or
|
||||
// or an authentication method is used (e.g. `passport.authenticate`)
|
||||
setup.getARouteHandler().(DataFlow::CallNode).getCalleeName() = "authenticate"
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `handler` is regenerates it's session using `req.session.regenerate`.
|
||||
*/
|
||||
pragma[inline]
|
||||
predicate regeneratesSession(Express::RouteSetup setup) {
|
||||
exists(
|
||||
setup
|
||||
.getARouteHandler()
|
||||
.(Express::RouteHandler)
|
||||
.getARequestSource()
|
||||
.ref()
|
||||
.getAPropertyRead("session")
|
||||
.getAPropertyRead("regenerate")
|
||||
)
|
||||
}
|
||||
|
||||
from Express::RouteSetup setup
|
||||
where
|
||||
isLoginSetup(setup) and
|
||||
not regeneratesSession(setup)
|
||||
select setup, "Route handler does not invalidate session following login"
|
||||
@@ -0,0 +1,18 @@
|
||||
const express = require('express');
|
||||
const session = require('express-session');
|
||||
var bodyParser = require('body-parser')
|
||||
const app = express();
|
||||
app.use(bodyParser.urlencoded({ extended: false }))
|
||||
app.use(session({
|
||||
secret: 'keyboard cat'
|
||||
}));
|
||||
|
||||
app.post('/login', function (req, res) {
|
||||
// Check that username password matches
|
||||
if (req.body.username === 'admin' && req.body.password === 'admin') {
|
||||
req.session.authenticated = true;
|
||||
res.redirect('/');
|
||||
} else {
|
||||
res.redirect('/login');
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,24 @@
|
||||
const express = require('express');
|
||||
const session = require('express-session');
|
||||
var bodyParser = require('body-parser')
|
||||
const app = express();
|
||||
app.use(bodyParser.urlencoded({ extended: false }))
|
||||
app.use(session({
|
||||
secret: 'keyboard cat'
|
||||
}));
|
||||
|
||||
app.post('/login', function (req, res) {
|
||||
// Check that username password matches
|
||||
if (req.body.username === 'admin' && req.body.password === 'admin') {
|
||||
req.session.regenerate(function (err) {
|
||||
if (err) {
|
||||
res.send('Error');
|
||||
} else {
|
||||
req.session.authenticated = true;
|
||||
res.redirect('/');
|
||||
}
|
||||
});
|
||||
} else {
|
||||
res.redirect('/login');
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user