Skip to content

Commit

Permalink
Merge pull request #1750 from o1-labs/audit/main-mayusetoken
Browse files Browse the repository at this point in the history
[main] Fix MayUseToken vulnerability in AccountUpdates
  • Loading branch information
MartinMinkov authored Jul 17, 2024
2 parents 885b50e + fa3d952 commit 046ec18
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 30 deletions.
54 changes: 26 additions & 28 deletions src/lib/mina/account-update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { Memo } from '../../mina-signer/src/memo.js';
import {
Events as BaseEvents,
Actions as BaseActions,
MayUseToken as BaseMayUseToken,
} from '../../bindings/mina-transaction/transaction-leaves.js';
import { TokenId as Base58TokenId } from './base58-encodings.js';
import {
Expand Down Expand Up @@ -131,7 +132,30 @@ type AuthRequired = Types.Json.AuthRequired;
type AccountUpdateBody = Types.AccountUpdate['body'];
type Update = AccountUpdateBody['update'];

type MayUseToken = AccountUpdateBody['mayUseToken'];
type MayUseToken = BaseMayUseToken;
const MayUseToken = {
type: BaseMayUseToken,
No: {
parentsOwnToken: Bool(false),
inheritFromParent: Bool(false),
},
ParentsOwnToken: {
parentsOwnToken: Bool(true),
inheritFromParent: Bool(false),
},
InheritFromParent: {
parentsOwnToken: Bool(false),
inheritFromParent: Bool(true),
},
isNo: ({
body: {
mayUseToken: { parentsOwnToken, inheritFromParent },
},
}: AccountUpdate) => parentsOwnToken.or(inheritFromParent).not(),
isParentsOwnToken: (a: AccountUpdate) => a.body.mayUseToken.parentsOwnToken,
isInheritFromParent: (a: AccountUpdate) =>
a.body.mayUseToken.inheritFromParent,
};

type Events = BaseEvents;
const Events = {
Expand Down Expand Up @@ -1200,33 +1224,7 @@ class AccountUpdate implements Types.AccountUpdate {
return Provable.witnessAsync(combinedType, compute);
}

static get MayUseToken() {
return {
type: provablePure({ parentsOwnToken: Bool, inheritFromParent: Bool }),
No: { parentsOwnToken: Bool(false), inheritFromParent: Bool(false) },
ParentsOwnToken: {
parentsOwnToken: Bool(true),
inheritFromParent: Bool(false),
},
InheritFromParent: {
parentsOwnToken: Bool(false),
inheritFromParent: Bool(true),
},
isNo({
body: {
mayUseToken: { parentsOwnToken, inheritFromParent },
},
}: AccountUpdate) {
return parentsOwnToken.or(inheritFromParent).not();
},
isParentsOwnToken(a: AccountUpdate) {
return a.body.mayUseToken.parentsOwnToken;
},
isInheritFromParent(a: AccountUpdate) {
return a.body.mayUseToken.inheritFromParent;
},
};
}
static MayUseToken = MayUseToken;

/**
* Returns a JSON representation of only the fields that differ from the
Expand Down
20 changes: 20 additions & 0 deletions src/lib/mina/account-update.unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
AccountUpdate,
PrivateKey,
Field,
Bool,
Mina,
Int64,
Types,
Expand All @@ -20,6 +21,14 @@ function createAccountUpdate() {
return accountUpdate;
}

function createAccountUpdateWithMayUseToken(
mayUseToken: AccountUpdate['body']['mayUseToken']
) {
let accountUpdate = AccountUpdate.defaultAccountUpdate(address);
accountUpdate.body.mayUseToken = mayUseToken;
return accountUpdate;
}

// can convert account update to fields consistently
{
let accountUpdate = createAccountUpdate();
Expand Down Expand Up @@ -122,3 +131,14 @@ function createAccountUpdate() {
'Check signature: Invalid signature on fee payer for key'
);
}

// correctly identifies when neither flag is set
{
let accountUpdate = createAccountUpdateWithMayUseToken({
parentsOwnToken: Bool(false),
inheritFromParent: Bool(false),
});
expect(AccountUpdate.MayUseToken.isNo(accountUpdate).toBoolean()).toEqual(
true
);
}
20 changes: 20 additions & 0 deletions src/lib/testing/random.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ZkappUri,
PublicKey,
StateHash,
MayUseToken,
} from '../../bindings/mina-transaction/transaction-leaves-bigint.js';
import { genericLayoutFold } from '../../bindings/lib/from-layout.js';
import { jsLayout } from '../../bindings/mina-transaction/gen/js-layout.js';
Expand Down Expand Up @@ -99,6 +100,23 @@ const authRequired = map(
),
AuthRequired.fromJSON
);
const mayUseToken = map(
oneOf<Json.MayUseToken[]>(
{
parentsOwnToken: true,
inheritFromParent: false,
},
{
parentsOwnToken: false,
inheritFromParent: true,
},
{
parentsOwnToken: false,
inheritFromParent: false,
}
),
MayUseToken.fromJSON
);
const tokenSymbolString = reject(
string(nat(tokenSymbolLength)),
(s) => stringLengthInBytes(s) > 6
Expand Down Expand Up @@ -145,6 +163,7 @@ const Generators: Generators = {
string: base58(nat(50)), // TODO replace various strings, like signature, with parsed types
number: nat(3),
TransactionVersion: uint32,
MayUseToken: mayUseToken,
};
let typeToBigintGenerator = new Map<Signable<any, any>, Random<any>>(
[TypeMap, primitiveTypeMap, customTypes]
Expand Down Expand Up @@ -255,6 +274,7 @@ const JsonGenerators: JsonGenerators = {
string: base58(nat(50)),
number: nat(3),
TransactionVersion: json_.uint32,
MayUseToken: mapWithInvalid(mayUseToken, MayUseToken.toJSON),
};
let typeToJsonGenerator = new Map<Signable<any, any>, Random<any>>(
[TypeMap, primitiveTypeMap, customTypes]
Expand Down
2 changes: 1 addition & 1 deletion src/mina

0 comments on commit 046ec18

Please sign in to comment.