JS: decoding JWT without signature verification

[am0o0]

[JarLob]

No-verification query should be under CWE-347.

[am0o0]

@JarLob I created one query file for each JWT package, but these all can be merged in some cases where the JWT is decoded in an unsafe way by package1 and then the signature has been validated by package2. it is easy to merge all these together because all have the same template ( only two unverifiedDecode and verifiedDecode predicated are different). I tried my best to have a full cover of examples and tests.

[am0o0]

Hi @JarLob I added a new query with the name JsonWebTokenLocalSource.ql which works with the strapi CVE and patch correctly.

[JarLob]

@am0o0 Could you please change the pull request from draft to ready for review, so someone from JavaScript CodeQL team can take a look?

[erik-krogh]

First you need to move everything into the experimental/ folders.

I'll do a review after you've done that.

[github-actions[bot]]

QHelp previews:

javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerification.qhelp

JWT missing secret or public key verification

A JSON Web Token (JWT) is used for authenticating and managing users in an application.

Only Decoding JWTs without checking if they have a valid signature or not can lead to security vulnerabilities.

Recommendation

Don't use methods that only decode JWT, Instead use methods that verify the signature of JWT.

Example

In the following code, you can see the proper usage of the most popular JWT libraries.

const express = require('express')
const app = express()
const jwtJsonwebtoken = require('jsonwebtoken');
const jwt_decode = require('jwt-decode');
const jwt_simple = require('jwt-simple');
const jose = require('jose')
const port = 3000

function getSecret() {
    return "A Safe generated random key"
}

app.get('/jose1', async (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: with signature verification
    await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret()))
})

app.get('/jose2', async (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: first without signature verification then with signature verification for same UserToken
    jose.decodeJwt(UserToken)
    await jose.jwtVerify(UserToken, new TextEncoder().encode(getSecret()))
})

app.get('/jwtSimple1', (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: first without signature verification then with signature verification for same UserToken
    jwt_simple.decode(UserToken, getSecret(), false);
    jwt_simple.decode(UserToken, getSecret());
})

app.get('/jwtSimple2', (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: with signature verification
    jwt_simple.decode(UserToken, getSecret(), true);
    jwt_simple.decode(UserToken, getSecret());
})

app.get('/jwtJsonwebtoken1', (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: with signature verification
    jwtJsonwebtoken.verify(UserToken, getSecret())
})

app.get('/jwtJsonwebtoken2', (req, res) => {
    const UserToken = req.headers.authorization;
    // GOOD: first without signature verification then with signature verification for same UserToken
    jwtJsonwebtoken.decode(UserToken)
    jwtJsonwebtoken.verify(UserToken, getSecret())
})


app.listen(port, () => {
    console.log(`Example app listening on port ${port}`)
})

In the following code, you can see the improper usage of the most popular JWT libraries.

const express = require('express')
const app = express()
const jwtJsonwebtoken = require('jsonwebtoken');
const jwt_decode = require('jwt-decode');
const jwt_simple = require('jwt-simple');
const jose = require('jose')
const port = 3000

function getSecret() {
    return "A Safe generated random key"
}
app.get('/jose', (req, res) => {
    const UserToken = req.headers.authorization;
    // BAD: no signature verification
    jose.decodeJwt(UserToken)
})

app.get('/jwtDecode', (req, res) => {
    const UserToken = req.headers.authorization;
    // BAD: no signature verification
    jwt_decode(UserToken)
})

app.get('/jwtSimple', (req, res) => {
    const UserToken = req.headers.authorization;
    // jwt.decode(token, key, noVerify, algorithm)
    // BAD: no signature verification
    jwt_simple.decode(UserToken, getSecret(), true);
})

app.get('/jwtJsonwebtoken', (req, res) => {
    const UserToken = req.headers.authorization;
    // BAD: no signature verification
    jwtJsonwebtoken.decode(UserToken)
})

app.listen(port, () => {
    console.log(`Example app listening on port ${port}`)
})

References

• JWT claim has not been verified
• Common Weakness Enumeration: CWE-347.

[am0o0]

@erik-krogh sorry this PR was one of my first PRs and I didn't apply what I learnt from you and your colleagues to this PR. I hope the changes are acceptable now.

[erik-krogh]

The decodeJwtWithoutVerification.qlref and decodeJwtWithoutVerificationLocalSource.qlref tests are failing.
Could you rerun those and update the expected outputs.

[am0o0]

I forgot to update this branch, the tests are still OK. are you sure about failing?

[erik-krogh]

I forgot to update this branch, the tests are still OK. are you sure about failing?

Yeah, they're passing now 🤷

The decodeJwtWithoutVerificationDoesNotWork.ql file still needs to be deleted though.

I think changing the from-where-select to the below might fix your issue (it's the fix I described further up):

from ConfigurationUnverifiedDecode cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where
  cfg.hasFlowPath(source, sink) and
  not exists(ConfigurationVerifiedDecode cfg2 |
    cfg2.hasFlowPath(any(DataFlow::PathNode p | p.getNode() = source.getNode()), _)
  )
select source.getNode(), source, sink, "Decoding JWT $@.", sink.getNode(),
  "without signature verification"

If that works you can use the new approach in your queries.

[erik-krogh]

Thanks. Everything looks good for a merge into experimental/ now 👍

Thanks again for the contribution.