Skip to content

Commit

Permalink
also deprecate getUnconstrained
Browse files Browse the repository at this point in the history
  • Loading branch information
mitschabaude committed Aug 22, 2024
1 parent 21e4940 commit a8df62c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Deprecated

- `this.sender.getAndRequireSignature()` deprecated in favor of `getAndRequireSignatureV2()` due to a vulnerability https://github.com/o1-labs/o1js/pull/1799
- `this.sender.getAndRequireSignature()` / `getUnconstrained()` deprecated in favor of `V2` versions due to a vulnerability https://github.com/o1-labs/o1js/pull/1799

### Fixes

Expand Down
37 changes: 24 additions & 13 deletions src/lib/mina/zkapp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -875,14 +875,10 @@ super.init();
sender = {
self: this as SmartContract,
/**
* The public key of the current transaction's sender account.
*
* Throws an error if not inside a transaction, or the sender wasn't passed in.
*
* **Warning**: The fact that this public key equals the current sender is not part of the proof.
* A malicious prover could use any other public key without affecting the validity of the proof.
*
* Consider using `this.sender.getAndRequireSignatureV2()` if you need to prove that the sender controls this account.
* @deprecated
* Deprecated in favor of `this.sender.getUnconstrainedV2()`.
* This method is vulnerable because it allows the prover to return a dummy (empty) public key,
* which would cause an account update with that public key to not be included.
*/
getUnconstrained(): PublicKey {
// TODO this logic now has some overlap with this.self, we should combine them somehow
Expand All @@ -902,6 +898,24 @@ super.init();
}
},

/**
* The public key of the current transaction's sender account.
*
* Throws an error if not inside a transaction, or the sender wasn't passed in.
*
* **Warning**: The fact that this public key equals the current sender is not part of the proof.
* A malicious prover could use any other public key without affecting the validity of the proof.
*
* Consider using `this.sender.getAndRequireSignatureV2()` if you need to prove that the sender controls this account.
*/
getUnconstrainedV2(): PublicKey {
let sender = this.getUnconstrained();
// we prove that the returned public key is not the empty key, in which case
// `createSigned()` would skip adding the account update, and nothing is proved
sender.x.assertNotEquals(0);
return sender;
},

/**
* @deprecated
* Deprecated in favor of `this.sender.getAndRequireSignatureV2()`.
Expand All @@ -916,14 +930,11 @@ super.init();
/**
* Return a public key that is forced to sign this transaction.
*
* Note: This doesn't prove that the return value is the transaction sender, but it does prove that whoever created
* Note: This doesn't prove that the return value is the transaction sender, but it proves that whoever created
* the transaction controls the private key associated with the returned public key.
*/
getAndRequireSignatureV2(): PublicKey {
let sender = this.getUnconstrained();
// we prove that the returned public key is not the empty key, in which case `createSigned()` would
// skip adding the account update
sender.x.assertNotEquals(0);
let sender = this.getUnconstrainedV2();
AccountUpdate.createSigned(sender);
return sender;
},
Expand Down

0 comments on commit a8df62c

Please sign in to comment.