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

Security Vulnerability: Lack of Access Token Signature Verification #687

Open
GuillemPM opened this issue Jan 28, 2024 · 8 comments
Open
Assignees
Labels
bug Something isn't working priority

Comments

@GuillemPM
Copy link
Collaborator

Description

Overview

Upon user login to the Reduced.to platform, an access_token is generated and stored as an HttpOnly cookie. However, the access_token lacks signature verification, enabling an attacker to manipulate the JWT token's payload. Exploiting this vulnerability allows unauthorized users to elevate their privileges by modifying the access_token cookie, granting them access to protected features, such as those restricted to ADMIN roles.

Steps to Reproduce

  1. Log in to Reduced.to.
  2. Obtain the access_token cookie.
  3. Decode the access_token payload.
  4. Modify the payload, specifically changing the role from USER to ADMIN.
  5. Update the access_token cookie with the manipulated payload.

Expected Behavior

Access tokens should be securely signed to prevent tampering. Any attempt to modify the token payload should result in invalidation of the token.

Actual Behavior

The access_token lacks signature verification, allowing an attacker to modify the payload and update the access_token cookie, thereby gaining unauthorized access to elevated roles and protected features.

Proposed Solution

Implement JWT signature verification for access/refresh tokens on server side to ensure their integrity and prevent tampering.

Screenshots

security1
security2
security3
security4

Additional information

No response

@GuillemPM GuillemPM added bug Something isn't working triage Needs triage priority labels Jan 28, 2024
@origranot
Copy link
Owner

origranot commented Jan 30, 2024

Hey @GuillemPM, I am so happy to hear from you!

You are right, there is no jwt signature check on the Qwik server.
Although we are using the backend (nest) as our API and for database access, we do check the jwt over there.
image

You are right, visually you have an "admin" access but you can't do nothing with it.
I think we can solve it using an internal shared library that handle authentication and JWT operation for both Qwik server and backend.

What do you think, do you want to work on that issue?

@ymoukhli
Copy link
Contributor

ymoukhli commented Feb 1, 2024

Hello @origranot,

i can work on this issue if no one wants it.

@origranot
Copy link
Owner

Hello @origranot,

i can work on this issue if no one wants it.

@ymoukhli, Hey,
That's sounds good, I assigned you to this issue 💯

@origranot origranot removed the triage Needs triage label Feb 1, 2024
@ymoukhli ymoukhli removed their assignment Feb 4, 2024
@GuillemPM
Copy link
Collaborator Author

GuillemPM commented Feb 12, 2024

@origranot
I've reviewed PR #698 and identified an issue with the frontend relying on the JWT payload to determine which menus are loaded. Here are two suggestions to address this:

  • Ensure the cookie is updated from the server-side with each request. This will ensure that the cookie always reflects the latest user context from the server, reducing the risk of tampered payloads. However, this approach may increase server-side bandwidth usage.

  • Instead of using the cookie JWT payload to display hardcoded data (menus), we should rely solely on the server. When loading the profile page, menu tabs should be fetched from the server API. This way, the server and database become the sole trusted sources of data. Then we should discuss how to handle this scenario, might be creating a table in database to map pages/menus to user roles... Might be an easy modifiable JSON file parsed in backend...

  • (My preferred solution) Fetch the role of the user when loading the page instead of relying on the cookie JWT payload, then show the menus according to that role, if we decide to use this method we wouldn't need to modify the database and access of pages would be handled by frontend.

I can help to extend more the idea of the preferred solution if needed.

@rachelyWinter
Copy link

Hey @origranot
Has this issue been addressed yet?
It looks like the issue has not been assigned to anyone...
I can work on it if it's relevant

@origranot
Copy link
Owner

Hey @origranot Has this issue been addressed yet? It looks like the issue has not been assigned to anyone... I can work on it if it's relevant

Sounds good, go for it.

@rachelyWinter
Copy link

@origranot
Several solutions have been proposed here
Should I solve with the solution that @GuillemPM proposed and preferred?

(Fetch the role of the user when loading the page instead of relying on the cookie JWT payload, then show the menus according to that role, if we decide to use this method we wouldn't need to modify the database and access of pages would be handled by frontend.)

@origranot
Copy link
Owner

@origranot Several solutions have been proposed here Should I solve with the solution that @GuillemPM proposed and preferred?

(Fetch the role of the user when loading the page instead of relying on the cookie JWT payload, then show the menus according to that role, if we decide to use this method we wouldn't need to modify the database and access of pages would be handled by frontend.)

That's a good idea. I think we should do it as a "server action" and fetch it before the page loads.
What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority
Projects
None yet
Development

No branches or pull requests

4 participants