Skip to content

Signed overflow paths return a wrapped garbage value beside the error selector; the inner-error path returns 0 #41

Closed

Description

@thedavidmeister

Unit

src/lib/parse/LibParseDecimal.sol:129 and :139 vs :121.

Violated property

Failure-value convention is inconsistent within one function. The inner-error path returns (selector, 0), but the function's own overflow paths return the wrapped cast next to the selector:

  • "57896044618658097711785492504343953926634992332820282019728792003956564819968" (2^255) → (ParseDecimalOverflow.selector, type(int256).min)
  • "-57896044618658097711785492504343953926634992332820282019728792003956564819969" (−(2^255+1)) → (ParseDecimalOverflow.selector, type(int256).max)
  • "--5" (garbage-in, documented as unchecked) → (ParseDecimalOverflow.selector, 25)

All three verified by execution against 35496d8. A caller that ignores the selector reads a plausible signed value that is wrong by sign and/or magnitude.

Triage framing

Might be intended: the documented contract (on the unsigned sibling) is selector-check-first, and zeroing costs gas on a path that is already an error. Might not: line 121 zeroes in the same function, so the convention is inconsistent, and the wrapped values are the most misleading possible ones (int256 min/max). The campaign's new testUnsafeStrToSignedIntBoundaryOverflow (PR #36) deliberately asserts only the selector so as not to enshrine either convention.

🤖 Generated with Claude Code

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

Metadata

Metadata

Labels

adversarialFound by adversarial testingauditAudit finding

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions