Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GHSA-h63v-hw6g-x8hp] An issue in the index.js decryptCookie function of cookie... #5067

Conversation

mathysEthical
Copy link

Updates

  • Affected products
  • CVSS v3
  • Severity
  • Source code location
  • Summary

Comments
Adding Title, CVSS score, product, source code URL

@github-actions github-actions bot changed the base branch from main to mathysEthical/advisory-improvement-5067 December 10, 2024 11:29
@darakian
Copy link
Contributor

Hi there @mathysEthical. Thanks for the PR, but can you help walk me through the attack here? Looking at your gist (which is secret?)
https://gist.github.com/mathysEthical/f45f1503f87381090e38a33c50eec971
also shared as an issue
ebourmalo/cookie-encrypter#9

The source reads as

const express = require('express');
const cookieParser = require('cookie-parser');
const cookieEncrypter = require('cookie-encrypter');
const app = express();

app.use(cookieParser("NicePasswordHereItIsAGoodSecret!"));
app.use(cookieEncrypter("NicePasswordHereItIsAGoodSecret!"));
 
app.get('/login', function(req, res) {
    res.cookie("role","guest")
    res.send("logged in as guest")
})

app.get("/admin",(req,res)=>{
    console.log(req.cookies)
    if(req.cookies.role=="admin"){
        res.send("Access granted.")
    }else{
        res.send("Access denied.")
    }
})

app.listen(80)

There's a claim that loading /login gives the user a cookie with the value

e:87c3aa62cf38214f7c25d66eacb4c95a:9df91af30fafe915f3ef71069653d4c1

and that 87c3aa62cf38214f7c25d66eacb4c95a is an IV

The write up then says We xor it by guest and by admin to change do the bit flip attack. I'm not sure I follow on what that means. The link provided introduces two new numbers as hex values and the origin of those numbers is also unclear to me. Can you help clarify?

@mathysEthical
Copy link
Author

Hello @darakian, sorry for the typos I just fixed them:
We XOR it by "guest" and by "admin" to do the bit flipping attack, here is a link to help

The two new HEX strings are the HEX representations of "guest" and "admin" followed by null bytes to prevent the XOR operation repeating.

Here is a link to get them

The gist was set to private to report the vulnerability, now that it has been published I just made it public 😄

If you have any other questions, feel free to ask 😄

@darakian
Copy link
Contributor

darakian commented Dec 10, 2024

The two new HEX strings are the HEX representations of "guest" and "admin" followed by null bytes to prevent the XOR operation repeating.

Oh I see. Literally the strings guest and admin as hex. Gotcha. Ok, so the attack is roughly

  1. Get a cookie from the system which embeds the guest string in the cookie itself.
  2. Grab the IV 87c3aa62cf38214f7c25d66eacb4c95a which is presented as is in the cookie
  3. XOR 87c3aa62cf38214f7c25d66eacb4c95a with the hex of guest to remove it and admin to add it
  4. Use new cookie which now has the admin string/role stored in it

That certainly reads like a cryptographic failure.

Tested it locally to verify as well and it worked. My local IV was 2bfb2d5833430308496df6b0f05d72a5 and looking at the source it does look like they're using a randomized IV.
https://github.com/ebourmalo/cookie-encrypter/blob/master/index.js#L36
But the same XOR approach worked 👍

I think this advisory could do with an expanded description though. Maybe the title could be something like Bit flip attack in cookie-encrypter or similar and the description could add something to the effect of due to a weakness in the encryption method used in cookie-encrypter an attack can use the world visible IV to edit encrypted cookies without decrypting the cookie itself.. I'm sure there's a name for this sort of attack as well which would be good to include (along with a reference maybe).

What do you think?

@mathysEthical
Copy link
Author

Yeah I like your suggestions, feel free to add them 😄
The attack is called AES CBC bit flipping and I think the CWE is CWE-325: https://cwe.mitre.org/data/definitions/325.html

@advisory-database advisory-database bot merged commit 56a3435 into mathysEthical/advisory-improvement-5067 Dec 11, 2024
2 checks passed
@advisory-database
Copy link
Contributor

Hi @mathysEthical! Thank you so much for contributing to the GitHub Advisory Database. This database is free, open, and accessible to all, and it's people like you who make it great. Thanks for choosing to help others. We hope you send in more contributions in the future!

@advisory-database advisory-database bot deleted the mathysEthical-GHSA-h63v-hw6g-x8hp branch December 11, 2024 21:35
@darakian
Copy link
Contributor

Right on. I've made a few edits to the merge on our end. Let me know if you're good with them 👍

@mathysEthical
Copy link
Author

Right on. I've made a few edits to the merge on our end. Let me know if you're good with them 👍

That's perfect ! Thanks @darakian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants