fix(ec): reject EC tags whose declared length underflows the children size - #879
Merged
Merged
Conversation
… size CECTag::ReadFromSocket() computes the per-tag payload length as tmp_len - GetTagLen(useLargeCount). Both operands are unsigned, and tmp_len is read straight from the wire while the children's serialized size is recovered post-parse from the children themselves. A malicious peer can craft a tag whose declared length is smaller than the size of its children, underflowing m_dataLen to ~0xFFFFFFFE and driving a multi-GB attacker-controlled allocation in NewData(). Tags are parsed while reading the authentication request, so this is reachable pre-auth on any node that exposes EC (default ECAddress is localhost; users that bind EC to a routable address expose a remote single-packet DoS). Add the missing lower-bound check before the subtraction and reject the tag. Complements the GetTagLen iteration cap added for amule-project#199. Closes amule-project#877.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jun 7, 2026
…ule-project#912) Extends existing categories (preferring extensions over new lines): - Performance/Upload: amule-project#898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with amule-project#879/amule-project#882/amule-project#890/amule-project#886; new amuleweb security hardening bullet consolidating ngosang's amule-project#869-amule-project#874 triage (all landed in amule-project#875); amulegui list extended with amule-project#857; shared-folder watcher extended with amule-project#858. - Packaging: Windows installer i18n line extended with amule-project#899. - Internals & Refactoring: new docs-polish + code-quality bullets covering amule-project#851/amule-project#855/amule-project#862/amule-project#888/amule-project#900/amule-project#866/amule-project#867/amule-project#895 and amule-project#909/amule-project#910/amule-project#912. - Translations: new pre-release final-wave bullet covering amule-project#847/amule-project#856/ amule-project#891/amule-project#908/amule-project#860/amule-project#904/amule-project#859/amule-project#863/amule-project#861/amule-project#880/amule-project#911/amule-project#901/amule-project#902/amule-project#889/amule-project#868/amule-project#853. - Bug Fixes & Stability: amule-project#850/amule-project#854/amule-project#878/amule-project#906. - CI: ccache wiring (amule-project#892, amule-project#903) + CodeQL binutils-dev (amule-project#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through amule-project#912.
mrjimenez
pushed a commit
that referenced
this pull request
Jun 8, 2026
Extends existing categories (preferring extensions over new lines): - Performance/Upload: #898 SlotAllocation default raised. - Networking & Discovery: wire-parser hardening list extended with #879/#882/#890/#886; new amuleweb security hardening bullet consolidating ngosang's #869-#874 triage (all landed in #875); amulegui list extended with #857; shared-folder watcher extended with #858. - Packaging: Windows installer i18n line extended with #899. - Internals & Refactoring: new docs-polish + code-quality bullets covering #851/#855/#862/#888/#900/#866/#867/#895 and #909/#910/#912. - Translations: new pre-release final-wave bullet covering #847/#856/ #891/#908/#860/#904/#859/#863/#861/#880/#911/#901/#902/#889/#868/#853. - Bug Fixes & Stability: #850/#854/#878/#906. - CI: ccache wiring (#892, #903) + CodeQL binutils-dev (#907). Contributors footer gains mifritscher and nguyenhoangminhhieu2004-gif (both first-time contributors). PR index extended through #912.
mrjimenez
pushed a commit
to mrjimenez/amule
that referenced
this pull request
Aug 10, 2026
…mule-project#879) CECTag's double constructor serialises through a default ostringstream, whose precision is 6 significant digits. That is fine for the small magnitudes most double tags carry (rates, ratios), but the statsgraph timestamp is seconds of uptime, and once it passes 1e6 the stream switches to scientific notation and quantises the value: a 10-second grid after ~11.6 days of uptime, 100 seconds after ~115 days. The statistics graph uses that timestamp as a delta marker. The client stores the newest point's timestamp and sends it back as EC_TAG_STATSGRAPH_LAST; the core returns everything newer. Quantised in both directions, the marker is wrong by up to a full grid step, so the core either resends points the client already has or skips points it never sent -- duplicated or missing samples on every refresh, on any daemon up more than ~11.6 days. Serialise with max_digits10 instead, the shortest precision that round-trips a double exactly. The tag stays a length-prefixed NUL-terminated string, so the longer text is compatible in both directions; GetDoubleData() parses it with istringstream unchanged.
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.
Fixes #877.
CECTag::ReadFromSocket()computes the per-tag payload length astmp_len - GetTagLen(useLargeCount). Both operands are unsigned, andtmp_lenis read straight from the wire while the children's serialized size is recovered post-parse from the children themselves. A malicious peer can craft a tag whose declared length is smaller than the size of its children, underflowingm_dataLento ~0xFFFFFFFEand driving a multi-GB attacker-controlled allocation inNewData().Tags are parsed while reading the authentication request, so this is reachable pre-auth on any node that exposes EC (default
ECAddressis localhost; users who bind EC to a routable address expose a remote single-packet DoS).Add the missing lower-bound check before the subtraction and reject the tag. Complements the
GetTagLeniteration cap added for #199.Test plan
Built clean on macOS local (Apple Silicon, Homebrew) and on Ubuntu ARM64. EC roundtrip via
amulecmdagainst anamuledfrom this branch:status,get bwlimits,show DL/UL/shared).set bwlimit down 100→ confirm → revert → confirm).reload shared).