JS: decoding JWT without signature verification

[am0o0]

No description provided.

[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]

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.

[erik-krogh]

A first review round.

Primarily: you shouldn't add one new query for each library you're adding a model for, you should just have one query that models all the libraries.

[erik-krogh]

This does not look like a file you meant to include?
This look like code you used for testing.

[am0o0]

I wrote this file for more clarification about what I wanted to do but I failed to express what I needed to get my query to the right point which I failed. this is what I want:
the same source has a Path to two different sinks, one is a dangerous sink, and the other is a guard sink. if one source has a path to both sinks then the source code doesn't contain any vulnerability, but if the this source only has a path to the dangerous sink then the source code contains a vulnerability and should detect it.

[erik-krogh]

Oh, I think I know why.
The DataFlow::PathNode includes which DataFlow::Configuration it came from.
So you need to check that the DataFlow::Node that you get from calling .getNode() on the source is the same between the two "sources", you can't use the DataFlow::PathNode from one configuration directly in another.

[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.