Skip to content

fix(ec): reject EC tags whose declared length underflows the children size - #879

Merged
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/ec-tag-len-underflow
Jun 6, 2026
Merged

fix(ec): reject EC tags whose declared length underflows the children size#879
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/ec-tag-len-underflow

Conversation

@got3nks

@got3nks got3nks commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #877.

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 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 GetTagLen iteration cap added for #199.

Test plan

Built clean on macOS local (Apple Silicon, Homebrew) and on Ubuntu ARM64. EC roundtrip via amulecmd against an amuled from this branch:

  • Read-side EC tag walk (status, get bwlimits, show DL / UL / shared).
  • Write-side EC tag round-trip (set bwlimit down 100 → confirm → revert → confirm).
  • Broader verb (reload shared).

… 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.
@mrjimenez
mrjimenez merged commit c8aed15 into amule-project:master Jun 6, 2026
7 checks passed
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.
@got3nks
got3nks deleted the fix/ec-tag-len-underflow branch June 8, 2026 10:33
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integer underflow → multi-GB allocation (DoS) in EC tag deserialization

2 participants