Skip to content

Conversation

@Oblivionsage
Copy link

Sets m_max_content_length to MAX_RPC_CONTENT_LENGTH (1MB) to match the protection already implemented in core_rpc_server.cpp (line 427).

Without this limit, wallet RPC accepts unlimited HTTP body sizes which could lead to resource exhaustion issues.

Testing:

  • Verified requests under 1MB work normally
  • Verified requests over 1MB are rejected
  • Behavior now consistent with core RPC server

Addresses private HackerOne security report

Set m_max_content_length to MAX_RPC_CONTENT_LENGTH (1MB) to match
the protection already implemented in core_rpc_server. This prevents
memory exhaustion attacks via unlimited HTTP request sizes.
@vtnerd
Copy link
Contributor

vtnerd commented Oct 3, 2025

I don't understand, someone would run wallet-rpc-server and allow remote connections?

@selsta
Copy link
Collaborator

selsta commented Oct 3, 2025

@vtnerd we deemed that this is not realistically exploitable, but if it's easy to add a limit I don't see why we shouldn't add one

@Oblivionsage do you know if for example import_key_images is implemented with batching? Otherwise this request can easily surpass 1MB.

@Oblivionsage
Copy link
Author

@selsta Good point. I don't know if import_key_images has batching implemented . If legitimate use cases regularly exceed 1MB, we might need either:

  1. A higher limit (like 10MB)
  2. Per-endpoint limits instead of a blanket 1MB

Core RPC uses the same 1MB limit - not sure if they have similar issues with large legitimate payloads. What would you suggest? I can adjust the limit or make it configurable if needed

@selsta
Copy link
Collaborator

selsta commented Oct 5, 2025

A per-endpoint limit is overkill here... I would set it to 100MB.

@Oblivionsage
Copy link
Author

@selsta updated per feedback. Let me know if this looks good or needs any changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants