From a8df62c57bb0d59b1a685ad61c59f4d3faae82fd Mon Sep 17 00:00:00 2001 From: Gregor Date: Thu, 22 Aug 2024 16:47:54 +0200 Subject: [PATCH] also deprecate getUnconstrained --- CHANGELOG.md | 2 +- src/lib/mina/zkapp.ts | 37 ++++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86a4b871f3..5390d99522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/lib/mina/zkapp.ts b/src/lib/mina/zkapp.ts index 30984a0bec..43a634cb91 100644 --- a/src/lib/mina/zkapp.ts +++ b/src/lib/mina/zkapp.ts @@ -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 @@ -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()`. @@ -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; },