Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bounce internal signed messages to protect relayer against abuse #14

Merged
merged 2 commits into from
Feb 20, 2024
Merged

Bounce internal signed messages to protect relayer against abuse #14

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2734f0
Bounce internal signed messages to protect relayer against abuse
oleganza Feb 20, 2024
b233fb7
fix throw argument
oleganza Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions contracts/wallet_v5.fc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,23 @@ cell verify_actions(cell c5) inline {
var stored_subwallet = ds~load_uint(size::stored_subwallet);
var public_key = ds.preload_uint(size::public_key);

;; Note on bouncing/nonbouncing behaviour:
;; In principle, the wallet should not bounce incoming messages as to avoid
;; returning deposits back to the sender due to opcode misinterpretation.
;; However, specifically for "gasless" transactions (signed messages relayed by a 3rd party),
;; there is a risk for the relaying party to be abused: their coins should be bounced back in case of a race condition or delays.
;; We resolve this dilemma by silently failing at the signature check (therefore ordinary deposits with arbitrary opcodes never bounce),
;; but failing with exception (therefore bouncing) after the signature check.

;; TODO: Consider moving signed into separate ref, slice_hash consumes 500 gas just like cell creation!
;; Only such checking order results in least amount of gas
return_unless(check_signature(slice_hash(signed), signature, public_key));
;; If public key is disabled, stored_seqno is strictly less than zero: stored_seqno < 0
;; However, msg_seqno is uint, therefore it can be only greater or equal to zero: msg_seqno >= 0
;; Thus, if public key is disabled, these two domains NEVER intersect, and additional check is not needed
return_unless(msg_seqno == stored_seqno);
return_unless(subwallet_id == stored_subwallet);
return_if(valid_until <= now());
throw_unless(33, msg_seqno == stored_seqno);
throw_unless(34, subwallet_id == stored_subwallet);
throw_if(36, valid_until <= now());

;; Store and commit the seqno increment to prevent replays even if the subsequent requests fail.
stored_seqno = stored_seqno + 1;
Expand Down Expand Up @@ -337,4 +345,4 @@ cell get_extensions() method_id {

int get_is_signature_auth_allowed() method_id {
return get_data().begin_parse().preload_int(size::stored_seqno) >= 0;
}
}
Loading