Repository navigation
Conversation
createuwreq judged the private-reserve rule only against the rows that existed at ingestion. A request naming a reserve that was missing, PENDING or CANCELLED passed, and that identity could become an ACTIVE private reserve before settlement (a new registration, matchreserve, or oncrtreserve's reclaim of a CANCELLED row). try_select_winner rechecked privacy only on the from-WIRE path. The rule is now one predicate, used wherever uwrit admits, drains or settles a swap; at race resolution a violation is final and the request is rejected and refunded before any lock or reserve write. try_build_swap_remit wrote the settlement quote into the signed TokenAmount.amount with a bare static_cast<int64_t>. opp::safe:: to_token_amount, the outbound mirror of to_depot_amount, now range-checks it after the request-level verdicts, rejecting and refunding a quote outside (0, depot_amount_max]. The path is unreachable today, because opreg caps every collateral bucket at asset::max_amount and the destination bond gate runs first; this is defense in depth. A UIC field declaring a length far past its input made zpp_bits resize before checking the remaining bytes, trapping on memory growth and reverting the whole consensus delivery. msgch and uwrit now decode UICs through opp::decode_uic, which refuses input over MAX_UIC_LEG_BYTES and caps every allocation at that bound; a compile-time check fails the build if the UIC tree gains a container the cap does not bound.
| } FC_LOG_AND_RETHROW() } | ||
|
|
||
| // createuwreq's reserve-privacy gate, against rows that exist at ingestion: a private reserve is | ||
| // refused as the source of a to-WIRE swap and against a counterpart that is public or has another |
Contributor
There was a problem hiding this comment.
The comment says a private reserve is refused against any public counterpart, but case 9903 uses PRIMARY, whose owner is empty, so it only verifies owner mismatch. The protocol permits a public counterpart when both reserves have the same non-empty Wire owner. Please reword this to “public and unowned” and add a positive same-owner public-counterpart case, so this test pins the intended owner-based policy instead of the stale flag-based interpretation.
The ingestion gate test described case 9903 as a private reserve refused against "a public counterpart", but PRIMARY has an empty owner, so the case only showed an owner mismatch. reserve_privacy_violation pairs a private reserve by owner, not by the counterpart's privacy flag: a public counterpart under the same non-empty WIRE owner is permitted. - Reword the test comment and case 9903 to "public and unowned". - Add regreserve_public_owned, registering a public reserve with an owner. - Ingestion: a public counterpart of the same owner is admitted (9905); one owned by another account is refused with the owner-mismatch reason (9906). - Settlement: a private source settles against a public destination of the same owner at race resolution, remitting the quote. Test-only; no contract or ABI change. Co-Authored-By: Claude Opus 5.5 <[email protected]> Change-Id: I4af2d35d6e11257a5ffc0dc654b74d6bb23e2d31
Contributor
|
The conflicts on master that have occurred since @heifner worked on this, and the rebase is a considerable change, so I decided that I would leave it for Kevin to fix on Monday, since I think he is going to be in the office I believe. |
Contributor
Author
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.
Three sysio.uwrit / sysio.msgch fixes from the CertiK Sysio Audit 1 preliminary report.
Private-reserve policy at settlement
createuwreqchecked the private-reserve rule only against rows that existed at ingestion, so a request naming a missing, PENDING or CANCELLED reserve could later settle against an ACTIVE private pool it was never allowed to use;try_select_winnerrechecked only the from-WIRE path. One predicate,reserve_privacy_violation, now coverscreateuwreq,try_select_winner,swapfromwireanddrainfwq. At race resolution every required reserve is ACTIVE and an ACTIVE reserve's privacy and owner never change, so a violation is final: reject and refund.The ingestion SWAP_REVERT reasons lose their
SwapRequest rejected:prefix (the text is now shared with settlement) and the em-dash becomes a colon. No consumer matches on them;swapfromwire's check message, which wire-tools-ts matches, is unchanged.SWAP_REMIT amount range
The remit amount was a bare
static_cast<int64_t>of the quote.opp::safe::to_token_amountnow gates it to(0, depot_amount_max], rejecting and refunding otherwise. The check runs intry_select_winnerafter the zero-quote verdicts, not intry_build_swap_remit, which runs while a zero ingestion quote must still leave the request PENDING. Unreachable today (opreg caps collateral atasset::max_amountand the bond gate runs first); the regression test forges a bond past the cap in chain state to reach it.Bounded UIC decoding
A forged length prefix trapped the allocator ("failed to allocate pages") and reverted the whole delivery.
opp::decode_uic(newsysio.opp.common/uic_codec.hpp) refuses input overMAX_UIC_LEG_BYTES, decodes withalloc_limit, and a compile-time walk requires every container in the UIC tree to be a byte string. msgch drops an oversized UIC before decoding it.Notes
sysio.uwrit.wasmandsysio.msgch.wasmare rebuilt; no ABI changes.createuwreq's SwapRequest decode have the same missing allocation limit (lower exposure, since outposts encode those payloads); sysio.opreg's outbound TokenAmount casts should go throughto_token_amountafter WIRE-339/WIRE-353: keep all four claim ledgers claimable indefinitely #639 lands.