Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

synapse blindly trusts X-Forwarded-For if x_forwarded option is enabled #9471

Open
richvdh opened this issue Feb 23, 2021 · 4 comments
Open
Labels
S-Minor Blocks non-critical functionality, workarounds exist. Security T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@richvdh
Copy link
Member

richvdh commented Feb 23, 2021

Synapse does not check that the chain in X-Forwarded-For is trusted, and so an attacker can spoof their IP address if the reverse proxy does not sanitize X-Forwarded-For. Ideally, we should be able to pass a set of trusted IP addresses, and synapse should only trust X-Forwarded-For if: 1) the request comes from a trusted IP address, and 2) every IP address in X-Forwarded-For, other than the first one, is trusted.

This can be mitigated by ensuring that the X-Forwarded-For header is sanitized before it hits synapse. For example, the public-facing reverse-proxy should remove any X-Forwarded-For header that it receives.

The IP address seems to be used for:

  • checking that AS requests come from trusted IP addresses
  • rate limiting registration requests
  • UI auth (maybe?)
  • request logging
  • last-seen IP address for devices
@richvdh
Copy link
Member Author

richvdh commented Feb 23, 2021

vaguely related: matrix-org/sydent#299

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Feb 23, 2021
@richvdh
Copy link
Member Author

richvdh commented Feb 23, 2021

I've just realised that our reverse proxy docs say nothing about sanitizing the X-Forwarded-For header, so this might be a bit more of an urgent security problem than I thought.

@richvdh
Copy link
Member Author

richvdh commented Feb 23, 2021

my suggested approach for fixing this would be to allow a trusted_proxies setting in the listener configuration, which is a list of IP addresses to be compared against the client IP and X-Forwarded-For addresses. x_forwarded can be deprecated.

@dklimpel
Copy link
Contributor

It is also related to matrix-org/matrix-content-scanner#36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Minor Blocks non-critical functionality, workarounds exist. Security T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

3 participants