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

new requirement: end-user should not be able to give and/or manipulate headers which an application expects/uses from load balancers or proxies #1697

Closed
elarlang opened this issue Jul 19, 2023 · 15 comments · Fixed by #1720
Assignees
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review Community wanted We would like feedback from the community to guide our decision otherwise we will progress _5.0 - prep This needs to be addressed to prepare 5.0

Comments

@elarlang
Copy link
Collaborator

elarlang commented Jul 19, 2023

Problem to solve: if an application uses HTTP request headers (like X-Forwarded-* headers) from load balancers, proxies etc, then it should be clear that those headers are not given by end user.

Fits to category: V14.5 HTTP Request Header Validation

@elarlang elarlang added the 1) Discussion ongoing Issue is opened and assigned but no clear proposal yet label Jul 19, 2023
@tghosth tghosth added the _5.0 - prep This needs to be addressed to prepare 5.0 label Sep 7, 2023
@tghosth
Copy link
Collaborator

tghosth commented Sep 7, 2023

Something like:

Verify that http headers which the application relies upon for business logic or security decisions such as X-Forwarded-* cannot be manipulated/controlled by the end-user.

What do you think @elarlang ?

@tghosth tghosth added the Community wanted We would like feedback from the community to guide our decision otherwise we will progress label Sep 7, 2023
@elarlang elarlang changed the title new requirement: end-user should not be able to give and/or manipulate headers which an application expects/users from load balancers or proxies new requirement: end-user should not be able to give and/or manipulate headers which an application expects/uses from load balancers or proxies Sep 7, 2023
@elarlang
Copy link
Collaborator Author

elarlang commented Sep 7, 2023

Can we use something like...

Verify that HTTP headers, which are set by middleware like proxy of load balancers, and application relies upon ...

End-users can manipulate those headers anyway, the point is, that this middle-layer should clean up headers with the same name.

@tghosth
Copy link
Collaborator

tghosth commented Sep 12, 2023

Are there any other security sensitive headers that you want to consider here or only those related to the original IP?

If there are other headers then this needs to stay as a separate requirement and we should be more specific maybe.

If the only headers we are talking about are those which provide source IP then I think we can merge with the requirement in #1389.

@jsulinski
Copy link
Contributor

jsulinski commented Sep 12, 2023

Not just IP, there are:

  • X-Forwarded-For
  • X-Forwarded-Host
  • X-Forwarded-Proto

Rails has historically struggled with these in certain configurations.

Perhaps something like:

Verify that HTTP headers defined by intermediary devices, such as X-Real-IP and X-Forwarded-*, that the application relies upon cannot be overridden by the end-user.

@elarlang
Copy link
Collaborator Author

I was thinking about it more and those (#1389) need to be separate requirements.

and/or to and in @jsulinski proposal and it's good for me.

@elarlang
Copy link
Collaborator Author

Requirement:
Verify that HTTP headers defined by intermediary devices, such as X-Real-IP and X-Forwarded-*, that the application relies upon cannot be overridden by the end-user.

Other pieces/proposals:

  • Category V14.5 HTTP Request Header Validation
  • Level: 2
  • CWE proposal CWE-345

@elarlang elarlang added the 4) proposal for review Issue contains clear proposal for add/change something label Sep 13, 2023
@tghosth
Copy link
Collaborator

tghosth commented Sep 13, 2023

My modification:

Verify that if the application relies upon HTTP headers such as X-Real-IP and X-Forwarded-*, which are defined by intermediary devices like load balancers or proxies, that these cannot be overridden by the end-user.

@elarlang
Copy link
Collaborator Author

One more thing - "relies upon" ... maybe just "uses"?

@tghosth
Copy link
Collaborator

tghosth commented Sep 13, 2023

Verify that if the application uses HTTP headers such as X-Real-IP and X-Forwarded-*, which are defined by intermediary devices like load balancers or proxies, that these cannot be overridden by the end-user.

Yeah ok. Any other comments @jsulinski ?

@elarlang elarlang added 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR and removed 4) proposal for review Issue contains clear proposal for add/change something labels Sep 14, 2023
elarlang added a commit that referenced this issue Sep 14, 2023
@elarlang elarlang linked a pull request Sep 14, 2023 that will close this issue
@elarlang elarlang added 6) PR awaiting review and removed 5) awaiting PR A proposal hs been accepted and reviewed and we are now waiting for a PR labels Sep 14, 2023
@jsulinski
Copy link
Contributor

Sounds good to me! Here's another version if you prefer, that resolves the awkwardness of 'which/that these':

Verify that any HTTP headers used by the application and defined by intermediary devices like load balancers or proxies, such as X-Real-IP and X-Forwarded-*, cannot be overridden by the end-user.

elarlang added a commit that referenced this issue Sep 15, 2023
@elarlang
Copy link
Collaborator Author

I updated PR based on that (097e6fa)

@tghosth
Copy link
Collaborator

tghosth commented Sep 18, 2023

@elarlang what do you think about CWE-346 instead of CWE-345?

@elarlang
Copy link
Collaborator Author

elarlang commented Sep 18, 2023

Both are ok, but what is the key difference?

  • CWE-345 - Insufficient Verification of Data Authenticity
    • The product does not sufficiently verify the origin or authenticity of data, in a way that causes it to accept invalid data.
  • CWE-346 - Origin Validation Error
    • The product does not properly verify that the source of data or communication is valid.

346 and Origin in web context associates with the Origin header.

From #1481 point of view, it does not matter too much anyway.

@tghosth
Copy link
Collaborator

tghosth commented Sep 18, 2023

@elarlang let's make it 346 anyway but I accept that this sort of thing is potentially going to become less important going forward

@elarlang
Copy link
Collaborator Author

(PR is updated, #1720)

tghosth pushed a commit that referenced this issue Sep 18, 2023
tghosth pushed a commit that referenced this issue Sep 18, 2023
elarlang added a commit to elarlang/ASVS that referenced this issue Nov 9, 2023
elarlang added a commit to elarlang/ASVS that referenced this issue Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1) Discussion ongoing Issue is opened and assigned but no clear proposal yet 6) PR awaiting review Community wanted We would like feedback from the community to guide our decision otherwise we will progress _5.0 - prep This needs to be addressed to prepare 5.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants