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

Solution apps a1 tictactoe - Broken Access Token #640

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

briggetteroman
Copy link

This solution refers to which of the apps?

A/M# - Broken Access Token - A1 - TicTacToe

What did you do to mitigate the vulnerability?

The SecDevLab reports that the TicTacToe application presents some vulnerabilities.
To mitigate these vulnerabilities, we add a function called verifyCurrentUser to control the user's access to their information and statistics. The function verifyCurrentUser verifies the information about the user from the JWT token ( username in the section payload, as we see in the image below) and compares the value username of the JWT token with the information filled in the parameter user from the request.

image

Did you test your changes? What commands did you run?

First, we reproduce the proof of concept of the attack. Then, we add a function to control access to users' information.

Reproduce the attack

To reproduce the attack, we start to create two users (user1 and user2) in the TicTacToe application. Then, I login to generate a JWT token for the user1 stored in the cookie with name tictacsession. Now, we copy the cookie value tictacsession=<JWT-token>.
image.

After that, we use the cookie value to execute the command to get statistics information, as we can see in the image below.

image

But, if I change the value of user parameter for user2 using the JWT token of the user1, we get statistics information of user2.
image

In the same way, we can update the results of each game using the cookie value.
image
Also, we can update the game results of other users using the cookies of user1.
image

Solution

We add the function verifyCurrentUser to check if the current user can execute the request.
Getting statistic information, we execute the command asking for user1 information using the user1 JWT token.
image
But, if we try to get user2 information with the user1 JWT token, we do have not authorization to get this information.
image
Updating the results of the game, we execute the command to update user1 game information using the user1 JWT token.
image
But, if we try to update user2 game information with the user1 JWT token, we do have not authorization to get this information
image

@gustavocovas gustavocovas added mitigation solution 🔒 This is a possible way to fix this vulnerability Tic-Tac-Toe labels Nov 11, 2024
@gustavocovas gustavocovas self-assigned this Nov 11, 2024
@gustavocovas
Copy link
Contributor

Good work, @briggetteroman!

Checking that the request user matches the session user fixes the broken access control. 🚀

I would like to suggest you to try and simplify the code further. Do we really need to supply the user as a parameter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mitigation solution 🔒 This is a possible way to fix this vulnerability Tic-Tac-Toe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants