fix(network): cap recv_file() cumulative bytes at 1 GiB#156
Merged
adequatelimited merged 1 commit intoaudit-fixesfrom Apr 13, 2026
Merged
fix(network): cap recv_file() cumulative bytes at 1 GiB#156adequatelimited merged 1 commit intoaudit-fixesfrom
adequatelimited merged 1 commit intoaudit-fixesfrom
Conversation
recv_file() had no upper limit on total bytes received. The loop appended OP_SEND_FILE packet payloads to the output file until the peer sent a short (EOF-signalling) packet or the connection dropped. A peer that streams full-size packets forever can fill the node's disk -- remote DoS. Added MAX_RECV_FILE_BYTES (1 GiB) in types.h and a cumulative `total` counter in recv_file(). When total exceeds the cap, the loop breaks and the existing error path deletes the partial file via fclose+remove, returning VERROR. 1 GiB is well above any legitimate single-file transfer the protocol performs (tfile.dat: tens-to- hundreds of MB; block files: <= 448 MB by protocol limits). Verified with a fork-based test harness (committed as .disable per the existing convention for reference test code not wired into `make test`). The harness forks a fake peer that streams OP_SEND_FILE indefinitely; the parent calls recv_file() and confirms VERROR return plus partial-file removal. Tested with cap temporarily lowered to 16 MiB for a fast cycle; cap restored to 1 GiB before commit. Closes #92
Collaborator
Author
|
Simple fix to count downloaded bytes from a potentially malicious peer to prevent them from sending perpetual byte streams when responding to legitimate data requests. Hard cap on all file transfers now limited to 1GB. No hard fork required to extend this if we start seeing larger file transfers in 20 years. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MAX_RECV_FILE_BYTES = 1 GiBinsrc/types.hrecv_file()now tracks cumulative received bytes and aborts the transfer + cleans up the partial file when the cap is exceeded.disableper the existing conventionProblem
recv_file()appended incomingOP_SEND_FILEpayloads to disk until the peer sent a short (EOF-signalling) packet or the connection dropped. A peer streaming full-size packets forever can fill the node's disk — remote DoS.Fix
1 GiB sits well above any legitimate file the protocol transfers (tfile.dat: tens-to-hundreds of MB on a mature chain; block files: ≤ 448 MB by protocol limits). The existing error path (
fclose(fp); remove(fname); return VERROR;) handles cleanup when the cap trips.Verification
Built a fork-based harness (
src/test/_f18-recv-file-limit.c.disable) that:send_tx/recv_tx, then streamsOP_SEND_FILEpackets with max-size buffers indefinitely (never sends a short EOF packet).callserver()+recv_file()against the fake peer, then inspects the return code and whether the partial file was deleted.Ran with the cap temporarily lowered to 16 MiB for a fast cycle (cap restored to 1 GiB before this commit). Result:
The harness is committed as a
.disablefile matching the existing repo convention for reference test code not integrated intomake test(seegettx-tag-resolve.c.disable,proof-checkproof.c.disable,proof-support.c.disable). It builds cleanly against the library and can be run manually by compiling againstlibmochimo.a.Test plan
Closes #92