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: stored xss via file upload #2258

Merged
merged 4 commits into from
Aug 22, 2023
Merged

Conversation

thamsimun
Copy link
Contributor

@thamsimun thamsimun commented Aug 14, 2023

Problem

Fixes VAPT vulnerability GTA-18-005 WP1: Stored XSS via file upload

More information about the vulnerability can be found here: https://www.notion.so/opengov/20230711-Go-VAPT-Documents-b3e410c4b7634f229de8c9b7af5dbb7d

Closes Stored XSS via file upload

Solution

  1. Eliminate the potential for users to control the uploaded file’s mime-type by setting the file.mimeType in request to be the mime type detected from FileType library

  2. If no mime type detected from FileType library, we look at file name extension and map it to a list of accepted mime type to prevent XSS - set the mime-type to plain text for all uploaded files the library is unable to detect except for .csv, .dwf and .dxf files.

    • csv: text/csv
    • dwf: application/x-dwf
    • dxf: application/dxf
    • others: text/plain

Tests

File types that cannot be detected by file type library

txt:
Screenshot 2023-08-14 at 7 38 14 PM

csv:
Screenshot 2023-08-14 at 7 39 03 PM
Screenshot 2023-08-14 at 7 39 12 PM

dwf:
Screenshot 2023-08-14 at 7 39 51 PM

dxf:
Screenshot 2023-08-14 at 7 40 37 PM

svg (with .txt extension) attack (should be opened with text/plain)

curl --location 'localhost:3000/api/user/url' \ --header 'Cookie: gogovsg=s%3AOJmJWpvX5i_UcFNVmA8q0Dw4ndire9Ks.lf36B%2FGGhiszvyvgCw%2FncM5vivQhxVoIWOAQ%2FDNKwKs; Cookie_1=value' \ --header 'Content-Type: image/svg+xml' \ --form 'file=@"/Users/thamsimun/Downloads/test-vapt-svg.txt"' \ --form 'shortUrl="test-svg-xss-attack"'

Screenshot 2023-08-14 at 7 41 44 PM Screenshot 2023-08-14 at 7 42 28 PM Screenshot 2023-08-14 at 7 42 50 PM

File that can be detected by file type library (check if they have same behaviour as current behaviour in prod/staging)

pdf:
Screenshot 2023-08-14 at 7 04 57 PM

docx:
Screenshot 2023-08-14 at 7 01 26 PM

jpg:
Screenshot 2023-08-14 at 7 10 27 PM

jpeg:
Screenshot 2023-08-14 at 7 13 03 PM

png:
Screenshot 2023-08-14 at 7 14 06 PM

avi:
Screenshot 2023-08-14 at 7 15 01 PM

bmp:

Screenshot 2023-08-14 at 7 18 09 PM

dwg:
Screenshot 2023-08-14 at 7 20 36 PM

gif:
Screenshot 2023-08-14 at 7 22 19 PM

mpeg:
Screenshot 2023-08-14 at 7 24 06 PM
Screenshot 2023-08-14 at 7 24 20 PM

mpg:
Screenshot 2023-08-14 at 7 25 37 PM
Screenshot 2023-08-14 at 7 25 49 PM

ods:
Screenshot 2023-08-14 at 7 27 06 PM

pptx:
Screenshot 2023-08-14 at 7 28 21 PM

Screenshot 2023-08-14 at 7 28 53 PM

rtf:
Screenshot 2023-08-14 at 7 29 57 PM
Screenshot 2023-08-14 at 7 29 43 PM

tif:
Screenshot 2023-08-14 at 7 34 35 PM
Screenshot 2023-08-14 at 7 34 30 PM

tiff:
Screenshot 2023-08-14 at 7 32 39 PM

Screenshot 2023-08-14 at 7 32 27 PM

xlsx:
Screenshot 2023-08-14 at 7 36 20 PM

Screenshot 2023-08-14 at 7 36 42 PM

zip:
Screenshot 2023-08-14 at 7 37 27 PM

@thamsimun thamsimun marked this pull request as draft August 14, 2023 07:22
@thamsimun thamsimun marked this pull request as ready for review August 14, 2023 15:18
Copy link
Contributor

@thanhdatle thanhdatle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@thamsimun thamsimun merged commit 801c4ae into develop Aug 22, 2023
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