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

fix(path-traversal): add regex to prevent path traversal attack #1828

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

seaerchin
Copy link
Contributor

Problem

Currently, our frontend is vulnerable to path traversal attacks due to non-sanitisation of path parameters.

Solution

Validate site name according to teh regex in the picture below. sanitization is not done for other stuff like resource room name because these allow spaces in the name.

Screenshot 2024-03-08 at 1 37 08 PM

Attempting to use %20 in the regex will lead to erroneous validation (as seen in the ss). We also cannot allow % because this will lead to allowing url encoded characters (%5c).
Screenshot 2024-03-08 at 1 38 41 PM

@timotheeg
Copy link
Contributor

It's strange to have a path-traversal fix done on frontend 🤔 . Typically path traversal is something that gets exercised (and fixed) on the backend (cause frontend can always be bypassed).

Can you describe the attack behaviour that was active, before that is this PR is meant to disable?

Copy link
Contributor Author

from teh VAPT report, what happened was that this was a frontend oriented attack, because the cure53 user had access to both repositories, so they were able to escape sites/a and go to sites/b and have the api call out.

normally on our backend, we have verifySiteMember which prevents this - hence thise is a FE oriented fix

@seaerchin seaerchin merged commit 23f4b35 into develop Mar 14, 2024
12 checks passed
@seaerchin seaerchin deleted the fix/path-traversal branch March 14, 2024 02:45
@dcshzj dcshzj mentioned this pull request Mar 14, 2024
3 tasks
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.

3 participants