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

proposal: 1.12.2 (50.5.2) move/merge to 12.5.2 (50.5.1) #1406

Closed
elarlang opened this issue Oct 21, 2022 · 25 comments · Fixed by #1413 or #2156
Closed

proposal: 1.12.2 (50.5.2) move/merge to 12.5.2 (50.5.1) #1406

elarlang opened this issue Oct 21, 2022 · 25 comments · Fixed by #1413 or #2156
Assignees
Labels
3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos V50 Group issues related to Web Frontend _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine.

Comments

@elarlang
Copy link
Collaborator

elarlang commented Oct 21, 2022

Related issue:

# Description L1 L2 L3 CWE
1.12.2 Verify that user-uploaded files - if required to be displayed or downloaded from the application - are served by either octet stream downloads, or from an unrelated domain, such as a cloud file storage bucket. Implement a suitable Content Security Policy (CSP) to reduce the risk from XSS vectors or other attacks from the uploaded file. 646

edit: CSP part is now removed from the requirement text.

CWE-646 - Reliance on File Name or Extension of Externally-Supplied File

Current requirement contains many parts:

  • are served by either octet stream downloads, or from an unrelated domain, such as a cloud file storage bucket
  • Implement a suitable Content Security Policy (CSP) to reduce the risk from XSS vectors.
    • Point: duplicates clearly Content-Security-Policy requirement
    • Covered by current 14.4.3
  • or other attacks from the uploaded file
    • Point: "other point" (none)
# Description L1 L2 L3 CWE
12.5.2 Verify that direct requests to uploaded files will never be executed as HTML/JavaScript content. 434
14.4.3 [MODIFIED] Verify that a Content Security Policy (CSP) response header is in place that helps mitigate impact for XSS attacks like HTML, DOM, CSS, JSON, and JavaScript injection vulnerabilities. 1021

CWE-434 - Unrestricted Upload of File with Dangerous Type

Proposals:

  • remove duplicate CSP part from the requirement (done, duplicate removed)
  • merge 1.12.2 to 12.5.2 as those serve the same goal
@elarlang elarlang self-assigned this Oct 21, 2022
elarlang added a commit that referenced this issue Nov 5, 2022
elarlang added a commit that referenced this issue Nov 5, 2022
@elarlang elarlang linked a pull request Nov 5, 2022 that will close this issue
tghosth pushed a commit that referenced this issue Nov 6, 2022
@elarlang elarlang reopened this Nov 6, 2022
@elarlang elarlang added the 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos label Nov 6, 2022
@elarlang elarlang changed the title proposal: cleanup 1.12.2 and move/merge to 12.5.2 proposal: 1.12.2 move/merge to 12.5.2 Nov 6, 2022
@tghosth
Copy link
Collaborator

tghosth commented Nov 21, 2022

Why did you reopen @elarlang?

@elarlang
Copy link
Collaborator Author

Why did you reopen @elarlang?

Only CSP part remove quick-fix is done, merge to make.

@tghosth tghosth added _5.0 - prep This needs to be addressed to prepare 5.0 josh/elar labels Dec 7, 2022
@tghosth
Copy link
Collaborator

tghosth commented Jan 25, 2023

Requirement history:

# Description L1 L2 L3 CWE
1.12.2 [MODIFIED] Verify that user-uploaded files - if required to be displayed or downloaded from the application - are served by either octet stream downloads, or from an unrelated domain, such as a cloud file storage bucket. 646
1.12.2 Verify that user-uploaded files - if required to be displayed or downloaded from the application - are served by either octet stream downloads, or from an unrelated domain, such as a cloud file storage bucket. Implement a suitable Content Security Policy (CSP) to reduce the risk from XSS vectors or other attacks from the uploaded file. 646
1.12.2 Verify that user-uploaded files - if required to be displayed or downloaded by the application - are served from an unrelated domain, such as a cloud file storage bucket or similar, to reduce the risk of image or other files exploiting XSS vectors via using content security policy and same origin policy restrictions. 646

The item was originally added in commit 7809c0b.

Based on the discussion in #455, it sounds like the main purpose was to promote sandboxing and not just for XSS but in general.

I would support keeping this as a separate requirement but moving it to section 12.5 as a practical/implementation requirement.

@elarlang
Copy link
Collaborator Author

Mapping it here: #1009

@jmanico
Copy link
Member

jmanico commented Feb 17, 2023

Suggest a small grammar change for 1.12.2 (please do not use dashes in requirements, they are not proper punctuation).

1.12.2 [MODIFIED] Verify that user-uploaded files, if required to be displayed or downloaded from the application, are served by either octet-stream downloads or an unrelated domain, such as a cloud file storage bucket.   646

@tghosth tghosth added 4a) Waiting for another This issue is waiting for another issue to be resolved and removed 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos labels May 28, 2023
@tghosth tghosth added 4b Major-rework These issues need to be part of a full chapter rework V12 labels Jul 10, 2023
elarlang pushed a commit to elarlang/ASVS that referenced this issue Dec 15, 2023
@elarlang elarlang changed the title proposal: 1.12.2 move/merge to 12.5.2 proposal: 1.12.2 (50.5.2) move/merge to 12.5.2 (50.5.1) Dec 15, 2023
@elarlang
Copy link
Collaborator Author

Update: those requirements are moved now to the Web Frontend Security category:

V50.5 Unintended Content Interpretation

# Description L1 L2 L3 CWE
50.5.1 [GRAMMAR, MOVED FROM 12.5.2] Verify that direct requests to uploaded files will never be executed as HTML and JavaScript content. 434
50.5.2 [MODIFIED, MOVED FROM 1.12.2] Verify that user-uploaded files - if required to be displayed or downloaded from the application - are served by either octet stream downloads, or from an unrelated domain, such as a cloud file storage bucket. 646

Proposal to merge them is still valid.

@tghosth
Copy link
Collaborator

tghosth commented Dec 28, 2023

So we now also have:

# Description L1 L2 L3 CWE
50.5.3 [ADDED, DEPRECATES 14.4.2] Verify that to prevent browsers from rendering content or functionality in HTTP responses in an incorrect context, security controls are in place (e.g. not serving the content unless headers indicate it is the correct context, Content-Security-Policy: sandbox, Content-Disposition: attachment, etc). For example when an API or other resource is loaded directly.

This seems to replace 50.5.1 definitely. What about 50.5.2?

@tghosth tghosth added V50 Group issues related to Web Frontend and removed V12 labels Jun 19, 2024
@jmanico
Copy link
Member

jmanico commented Oct 15, 2024

Agreed on 50.5.1, 50.5.2 and 50.5.3 merging. How about:

Verify that user-uploaded files are never executed as HTML or JavaScript, are served as octet stream downloads or from a separate domain, and implement controls (e.g., Content-Security-Policy: sandbox, Content-Disposition: attachment) to prevent incorrect rendering by browsers.

@elarlang
Copy link
Collaborator Author

elarlang commented Oct 15, 2024

Jim - you are cutting so many details and keep proposing a separate domain. Please read previous comments.

I prefer we just cover 50.5.1 and 50.5.2 points into current 50.5.3:

Verify that security controls are in place to prevent browsers from rendering content or functionality in HTTP responses in an incorrect context (e.g., when an API, a user-uploaded file or other resource is requested directly). Possible controls could include: not serving the content unless HTTP request headers, such as Sec-Fetch-*, indicate it is the correct context, Content-Security-Policy: sandbox, Content-Disposition: attachment, etc.

edit: finetuned the last sentense

@jmanico
Copy link
Member

jmanico commented Oct 15, 2024

Sorry @elarlang was moving too fast. I like your version. All is well.

@elarlang
Copy link
Collaborator Author

Tags (the idea: 12.5.2 was moved to 50.5.1 and other 2 requirements were merged into it):

  • to new merged requirement [MODIFIED, MOVED FROM 12.5.2, MERGED FROM 1.12.2, 14.4.2]
  • 12.5.2 [MOVED TO 50.5.1]
  • 1.12.2 [DELETED, MERGED TO 50.5.1]
  • 14.4.2 [DELETED, MERGED TO 50.5.1] - previously it the new requirement said "DEPRECATES 14.4.2"

@elarlang elarlang removed josh/elar next meeting Filter for leaders labels Oct 15, 2024
@elarlang
Copy link
Collaborator Author

ping @tghosth - please recheck before I hit the PR.

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Oct 15, 2024
@tghosth
Copy link
Collaborator

tghosth commented Oct 15, 2024

@elarlang that all makes sense to me

@elarlang elarlang added the 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR label Oct 16, 2024
elarlang pushed a commit to elarlang/ASVS that referenced this issue Oct 16, 2024
@elarlang elarlang added 6) PR awaiting review and removed 4) proposal for review Issue contains clear proposal for add/change something 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Oct 16, 2024
@tghosth tghosth reopened this Dec 16, 2024
@tghosth
Copy link
Collaborator

tghosth commented Dec 16, 2024

Current v50.6.1:

[MODIFIED, MOVED FROM 12.5.2, MERGED FROM 1.12.2, 14.4.2] Verify that security controls are in place to prevent browsers from rendering content or functionality in HTTP responses in an incorrect context (e.g., when an API, a user-uploaded file or other resource is requested directly). Possible controls could include: not serving the content unless HTTP request header fields, such as Sec-Fetch-*, indicate it is the correct context, Content-Security-Policy: sandbox, Content-Disposition: attachment, etc.

@elarlang you mentioned elsewhere that you might want to split this back up? What is your proposal?

@tghosth tghosth added 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos and removed 4b Major-rework These issues need to be part of a full chapter rework 6) PR awaiting review 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet labels Dec 16, 2024
@elarlang elarlang added _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine. and removed _5.0 - prep This needs to be addressed to prepare 5.0 labels Jan 2, 2025
@elarlang
Copy link
Collaborator Author

elarlang commented Jan 2, 2025

So, the theoretical need is to split it because it contains different details that may require a separate level for the requirement.

User-uploaded files and API responses containing user-controlled data are for sure level 1. If there are other pieces of content, such as some template files in a public folder, then accessing those is not that risky and can be considered a level 3 requirement.

If there is agreement, that the requirement can be level 1 as is, there is no need to split anything. The split is not easy to make without having close-to-duplicate requirements.

@tghosth
Copy link
Collaborator

tghosth commented Jan 5, 2025

Ok so let's leave it for now, I updated the Google Sheet

@tghosth tghosth closed this as completed Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3) awaiting proposal There is some discussion in issue and reach to some results but it's not concluded with clear propos V50 Group issues related to Web Frontend _5.0 - Not blocker This issue does not block 5.0 so if it gets addressed then great, if not then fine.
Projects
None yet
3 participants