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

Add option to server to disable JSON responses #6838

Open
nflaig opened this issue Jun 1, 2024 · 2 comments
Open

Add option to server to disable JSON responses #6838

nflaig opened this issue Jun 1, 2024 · 2 comments
Labels
scope-security Issues that fix security issues: DOS, key leak, CVEs.

Comments

@nflaig
Copy link
Member

nflaig commented Jun 1, 2024

Add option to server to disable JSON responses to avoid potential DoS vector.

From #5128 (comment) and moved out todos from #6749 as it is not required to be implemented as part of PR and requires more discussion / clarity on what are the expectations.

While this is quite simple to implement by updating SUPPORTED_MEDIA_TYPES

export const SUPPORTED_MEDIA_TYPES = Object.values(MediaType);

it is not clear to me if we can disable json globally, because

  • users might wanna query data as json (it's just simpler in a lot of cases)
  • some apis do not support ssz because it's not technically possible (no json to ssz mapping), essentially disabling those completely

Since the main goal of this is to make it more difficult to DoS / crash public Lodestar nodes, we might wanna limit this to just APIs like getStateV2 . There are also other offenders like getStateValidators which do not support ssz but can bring down the node quite easily.

@nflaig nflaig added the scope-security Issues that fix security issues: DOS, key leak, CVEs. label Jun 1, 2024
@nflaig nflaig changed the title Add option to server to disable JSON responses to avoid potential DoS vector Add option to server to disable JSON responses Jun 1, 2024
@nflaig
Copy link
Member Author

nflaig commented Jun 5, 2024

Thinking about this more, the main issue why JSON is a DoS vector is due to serialization (JSON.stringify) because it takes a lot of memory if you do this for the beacon state or a huge validator array. We don't necessarily need to disable JSON if we can figure out a way to avoid passing huge objects like the state into stringify and throw an error instead.

This would address both concerns brought up in the issue and even allow to still use getStateValidators but it has to be queried for a subset of validators by passing pubkeys, and for getStateV2 it would mean it can only return SSZ on holesky and mainnet but it can still returns JSON data in dev mode, smaller networks (e.g. sepolia), or kurtosis.

@nflaig
Copy link
Member Author

nflaig commented Jun 5, 2024

if we can figure out a way to avoid passing huge objects like the state into stringify and throw an error instead.

So far, I have not found a solution for this that is cheap enough to add to our api as it requires to iterate over the objects (or convert to a Buffer) which would be a DoS vector itself, and affect api latency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope-security Issues that fix security issues: DOS, key leak, CVEs.
Projects
None yet
Development

No branches or pull requests

1 participant