Merge pull request #606 from getwax/merge-in-contract-updates

Audit Fix 06 Integration branch
This commit is contained in:
Jake C-T
2023-06-28 15:31:43 -06:00
committed by GitHub
9 changed files with 36 additions and 288 deletions

View File

@@ -21,7 +21,7 @@
"@types/koa__cors": "^3.3.0",
"@types/koa__router": "^8.0.11",
"@types/node-fetch": "^2.6.1",
"bls-wallet-clients": "0.9.0-2a20bfe",
"bls-wallet-clients": "0.9.0-3251dec",
"fp-ts": "^2.12.1",
"io-ts": "^2.2.16",
"io-ts-reporters": "^2.0.1",

View File

@@ -569,16 +569,6 @@
version "5.7.0"
resolved "https://registry.yarnpkg.com/@ethersproject/transactions/-/transactions-5.7.0.tgz#91318fc24063e057885a6af13fdb703e1f993d3b"
integrity sha512-kmcNicCp1lp8qanMTC3RIikGgoJ80ztTyvtsFvCYpSCfkjhD0jZ2LOrnbcuxuToLIUYYf+4XwD1rP+B/erDIhQ==
dependencies:
"@ethersproject/address" "^5.7.0"
"@ethersproject/bignumber" "^5.7.0"
"@ethersproject/bytes" "^5.7.0"
"@ethersproject/constants" "^5.7.0"
"@ethersproject/keccak256" "^5.7.0"
"@ethersproject/logger" "^5.7.0"
"@ethersproject/properties" "^5.7.0"
"@ethersproject/rlp" "^5.7.0"
"@ethersproject/signing-key" "^5.7.0"
"@ethersproject/units@5.6.0":
version "5.6.0"
@@ -887,10 +877,10 @@ bech32@1.1.4:
resolved "https://registry.yarnpkg.com/bech32/-/bech32-1.1.4.tgz#e38c9f37bf179b8eb16ae3a772b40c356d4832e9"
integrity sha512-s0IrSOzLlbvX7yp4WBfPITzpAU8sqQcpsmwXDiKwrG4r491vwCO/XpejasRNl0piBMe/DvP4Tz0mIS/X1DPJBQ==
bls-wallet-clients@0.9.0-2a20bfe:
version "0.9.0-2a20bfe"
resolved "https://registry.yarnpkg.com/bls-wallet-clients/-/bls-wallet-clients-0.9.0-2a20bfe.tgz#2e39757a18df3ba78d816ae15f6b88000443a2a6"
integrity sha512-w4efcArPBEowrAkIdVYc2mOLlkN8E5O9eIqEhoo6IrRVrN21p/JVNdoot4N3o5MAKFbeaYfid/u9lL6p2DNdiw==
bls-wallet-clients@0.9.0-3251dec:
version "0.9.0-3251dec"
resolved "https://registry.npmjs.org/bls-wallet-clients/-/bls-wallet-clients-0.9.0-3251dec.tgz#46f6795110abd90a242fec5a59af1ae2a9f45cd9"
integrity sha512-GBjJToEK3nEkldnWB8971BlIKCF0AT78GsrnkXfC1WtAYsvtqyZxmfGABMmN4si8GgykyyjYcxTypORz61AGZQ==
dependencies:
"@thehubbleproject/bls" "^0.5.1"
ethers "^5.7.2"

View File

@@ -54,7 +54,7 @@ export type {
PublicKey,
Signature,
VerificationGateway,
} from "https://esm.sh/bls-wallet-clients@0.9.0-2a20bfe";
} from "https://esm.sh/bls-wallet-clients@0.9.0-3251dec";
export {
Aggregator as AggregatorClient,
@@ -70,10 +70,10 @@ export {
getConfig,
MockERC20Factory,
VerificationGatewayFactory,
} from "https://esm.sh/bls-wallet-clients@0.9.0-2a20bfe";
} from "https://esm.sh/bls-wallet-clients@0.9.0-3251dec";
// Workaround for esbuild's export-star bug
import blsWalletClients from "https://esm.sh/bls-wallet-clients@0.9.0-2a20bfe";
import blsWalletClients from "https://esm.sh/bls-wallet-clients@0.9.0-3251dec";
const { bundleFromDto, bundleToDto, initBlsWalletSigner } = blsWalletClients;
export { bundleFromDto, bundleToDto, initBlsWalletSigner };

View File

@@ -1,6 +1,6 @@
{
"name": "bls-wallet-clients",
"version": "0.9.0-2a20bfe",
"version": "0.9.0-3251dec",
"description": "Client libraries for interacting with BLS Wallet components",
"main": "dist/src/index.js",
"types": "dist/src/index.d.ts",

View File

@@ -157,21 +157,17 @@ contract VerificationGateway
@dev overrides previous wallet address registered with the given public key
@param messageSenderSignature signature of message containing only the calling address
@param publicKey that signed the caller's address
@param signatureExpiryTimestamp that the signature is valid until
*/
function setBLSKeyForWallet(
uint256[2] memory messageSenderSignature,
uint256[BLS_KEY_LEN] memory publicKey,
uint256 signatureExpiryTimestamp
uint256[BLS_KEY_LEN] memory publicKey
) public {
require(blsLib.isZeroBLSKey(publicKey) == false, "VG: key is zero");
IWallet wallet = IWallet(msg.sender);
bytes32 existingHash = hashFromWallet[wallet];
if (existingHash == bytes32(0)) { // wallet does not yet have a bls key registered with this gateway
// set it instantly
safeSetWallet(messageSenderSignature, publicKey, wallet, signatureExpiryTimestamp);
}
else { // wallet already has a key registered, set after delay
// Can't register new wallet contracts, only what this gateway deployed.
if (existingHash != bytes32(0)) { // wallet already has a key registered, set after delay
pendingMessageSenderSignatureFromHash[existingHash] = messageSenderSignature;
pendingBLSPublicKeyFromHash[existingHash] = publicKey;
pendingBLSPublicKeyTimeFromHash[existingHash] = block.timestamp + 604800; // 1 week from now
@@ -235,6 +231,11 @@ contract VerificationGateway
}
}
require((selectorId != ProxyAdmin.upgrade.selector)
&& (selectorId != ProxyAdmin.upgradeAndCall.selector),
"VG: wallet not upgradable"
);
wallet.setAnyPending();
// ensure wallet has pre-approved encodedFunction

View File

@@ -95,7 +95,7 @@ describe("Recovery", async function () {
wallet1,
vg,
"setBLSKeyForWallet",
[addressSignature, wallet2.PublicKey(), signatureExpiryTimestamp],
[addressSignature, wallet2.PublicKey()],
1,
30_000_000,
);
@@ -372,7 +372,7 @@ describe("Recovery", async function () {
wallet1,
vg,
"setBLSKeyForWallet",
[attackSignature, walletAttacker.PublicKey(), signatureExpiryTimestamp],
[attackSignature, walletAttacker.PublicKey()],
recoveredWalletNonce++,
30_000_000,
);
@@ -660,7 +660,7 @@ describe("Recovery", async function () {
wallet1,
vg,
"setBLSKeyForWallet",
[addressSignature, wallet2.PublicKey(), invalidSignatureExpiryTimestamp],
[addressSignature, wallet2.PublicKey()],
1,
30_000_000,
);
@@ -724,7 +724,7 @@ describe("Recovery", async function () {
wallet1,
vg,
"setBLSKeyForWallet",
[addressSignature, wallet2.PublicKey(), signatureExpiryTimestamp],
[addressSignature, wallet2.PublicKey()],
1,
30_000_000,
);

View File

@@ -1,21 +1,13 @@
import { expect } from "chai";
import { BigNumber, ContractReceipt } from "ethers";
import { solidityPack } from "ethers/lib/utils";
import { ethers, network } from "hardhat";
import { ethers } from "hardhat";
import { expectPubkeysEql } from "./expect";
import {
ActionData,
BlsWalletWrapper,
getOperationResults,
} from "../clients/src";
import { ActionData, getOperationResults } from "../clients/src";
import Fixture from "../shared/helpers/Fixture";
import {
proxyAdminBundle,
proxyAdminCall,
} from "../shared/helpers/callProxyAdmin";
import { proxyAdminCall } from "../shared/helpers/callProxyAdmin";
import getPublicKeyFromHash from "../shared/helpers/getPublicKeyFromHash";
import deploy from "../shared/deploy";
const expectOperationsToSucceed = (txnReceipt: ContractReceipt) => {
const opResults = getOperationResults(txnReceipt);
@@ -44,7 +36,7 @@ describe("Upgrade", async function () {
fx = await Fixture.getSingleton();
});
it("should upgrade wallet contract", async () => {
it("should NOT upgrade wallet contract", async () => {
const MockWalletUpgraded = await ethers.getContractFactory(
"MockWalletUpgraded",
);
@@ -57,243 +49,19 @@ describe("Upgrade", async function () {
wallet.address,
mockWalletUpgraded.address,
]);
expectOperationsToSucceed(txnReceipt1);
// Advance time one week
const latestTimestamp = (await ethers.provider.getBlock("latest"))
.timestamp;
await network.provider.send("evm_setNextBlockTimestamp", [
BigNumber.from(latestTimestamp)
.add(safetyDelaySeconds + 1)
.toHexString(),
]);
expectOperationFailure(txnReceipt1, "VG: wallet not upgradable");
// make call
const txnReceipt2 = await proxyAdminCall(fx, wallet, "upgrade", [
const txnReceipt2 = await proxyAdminCall(fx, wallet, "upgradeAndCall", [
wallet.address,
mockWalletUpgraded.address,
[],
]);
expectOperationsToSucceed(txnReceipt2);
const newBLSWallet = MockWalletUpgraded.attach(wallet.address);
await (await newBLSWallet.setNewData(wallet.address)).wait();
await expect(newBLSWallet.newData()).to.eventually.equal(wallet.address);
expectOperationFailure(txnReceipt2, "VG: wallet not upgradable");
});
it("should register with new verification gateway", async () => {
// Deploy new verification gateway
const [signer] = await ethers.getSigners();
const deployment2 = await deploy(
signer,
ethers.utils.solidityPack(["uint256"], [2]),
);
const vg2 = deployment2.verificationGateway;
// Recreate hubble bls signer
const walletOldVg = await fx.createBLSWallet();
const walletAddress = walletOldVg.address;
const blsSecret = walletOldVg.blsWalletSigner.privateKey;
// Sign simple address message
const walletNewVg = await BlsWalletWrapper.connect(
blsSecret,
vg2.address,
vg2.provider,
);
const signatureExpiryTimestamp =
(await fx.provider.getBlock("latest")).timestamp +
safetyDelaySeconds +
signatureExpiryOffsetSeconds;
const addressMessage = solidityPack(
["address", "uint256"],
[walletAddress, signatureExpiryTimestamp],
);
const addressSignature = walletNewVg.signMessage(addressMessage);
const proxyAdmin2Address = await vg2.walletProxyAdmin();
// Get admin action to change proxy
const bundle = await proxyAdminBundle(fx, walletOldVg, "changeProxyAdmin", [
walletAddress,
proxyAdmin2Address,
]);
const changeProxyAction = bundle.operations[0].actions[0];
// prepare call
const txnReceipt = await proxyAdminCall(
fx,
walletOldVg,
"changeProxyAdmin",
[walletAddress, proxyAdmin2Address],
);
expectOperationsToSucceed(txnReceipt);
// Advance time one week
await fx.advanceTimeBy(safetyDelaySeconds + 1);
const hash = walletOldVg.blsWalletSigner.getPublicKeyHash();
const setExternalWalletAction: ActionData = {
ethValue: BigNumber.from(0),
contractAddress: vg2.address,
encodedFunction: vg2.interface.encodeFunctionData("setBLSKeyForWallet", [
addressSignature,
walletOldVg.PublicKey(),
signatureExpiryTimestamp,
]),
};
const setTrustedBLSGatewayAction: ActionData = {
ethValue: BigNumber.from(0),
contractAddress: fx.verificationGateway.address,
encodedFunction: fx.verificationGateway.interface.encodeFunctionData(
"setTrustedBLSGateway",
[hash, vg2.address],
),
};
// Upgrading the gateway requires these three steps:
// 1. register external wallet in vg2
// 2. change proxy admin to that in vg2
// 3. lastly, set wallet's new trusted gateway
//
// If (1) or (2) are skipped, then (3) should fail, and therefore the whole
// operation should fail.
{
// Fail if setExternalWalletAction is skipped
const { successes } =
await fx.verificationGateway.callStatic.processBundle(
walletOldVg.sign({
nonce: BigNumber.from(1),
gas: BigNumber.from(30_000_000),
actions: [
// skip: setExternalWalletAction,
changeProxyAction,
setTrustedBLSGatewayAction,
],
}),
);
expect(successes).to.deep.equal([false]);
}
{
// Fail if changeProxyAction is skipped
const { successes } =
await fx.verificationGateway.callStatic.processBundle(
walletOldVg.sign({
nonce: BigNumber.from(1),
gas: BigNumber.from(30_000_000),
actions: [
setExternalWalletAction,
// skip: changeProxyAction,
setTrustedBLSGatewayAction,
],
}),
);
expect(successes).to.deep.equal([false]);
}
{
// Succeed if nothing is skipped
const { successes } =
await fx.verificationGateway.callStatic.processBundle(
walletOldVg.sign({
nonce: BigNumber.from(1),
gas: BigNumber.from(30_000_000),
actions: [
setExternalWalletAction,
changeProxyAction,
setTrustedBLSGatewayAction,
],
}),
);
expect(successes).to.deep.equal([true]);
}
await expect(vg2.walletFromHash(hash)).to.eventually.not.equal(
walletAddress,
);
// Now actually perform the upgrade so we can perform some more detailed
// checks.
await fx.processBundleWithExtraGas(
walletOldVg.sign({
nonce: BigNumber.from(1),
gas: BigNumber.from(30_000_000),
actions: [
setExternalWalletAction,
changeProxyAction,
setTrustedBLSGatewayAction,
],
}),
);
// Create required objects for data/contracts for checks
const proxyAdmin = await ethers.getContractAt(
"ProxyAdmin",
await vg2.walletProxyAdmin(),
);
// Direct checks corresponding to each action
await expect(vg2.walletFromHash(hash)).to.eventually.equal(walletAddress);
await expect(vg2.hashFromWallet(walletAddress)).to.eventually.equal(hash);
await expect(proxyAdmin.getProxyAdmin(walletAddress)).to.eventually.equal(
proxyAdmin.address,
);
expectPubkeysEql(
await getPublicKeyFromHash(vg2, hash),
walletOldVg.PublicKey(),
);
const blsWallet = await ethers.getContractAt("BLSWallet", walletAddress);
// New verification gateway pending
await expect(blsWallet.trustedBLSGateway()).to.eventually.equal(
fx.verificationGateway.address,
);
// Advance time one week
await fx.advanceTimeBy(safetyDelaySeconds + 1);
// set pending
await (await blsWallet.setAnyPending()).wait();
// Check new verification gateway was set
await expect(blsWallet.trustedBLSGateway()).to.eventually.equal(
vg2.address,
);
await walletNewVg.syncWallet(vg2);
// Check new gateway has wallet via static call through new gateway
const bundleResult = await vg2.callStatic.processBundle(
fx.blsWalletSigner.aggregate([
walletNewVg.sign({
nonce: BigNumber.from(2),
gas: BigNumber.from(30_000_000),
actions: [
{
ethValue: 0,
contractAddress: vg2.address,
encodedFunction: vg2.interface.encodeFunctionData(
"walletFromHash",
[hash],
),
},
],
}),
]),
);
const walletFromHashAddress = ethers.utils.defaultAbiCoder.decode(
["address"],
bundleResult.results[0][0], // first and only operation/action result
)[0];
expect(walletFromHashAddress).to.equal(walletAddress);
// Still possible to point wallets to a new gateway if desired, just not with v1 deployment
});
it("should change mapping of an address to hash", async () => {
@@ -347,7 +115,6 @@ describe("Upgrade", async function () {
encodedFunction: vg1.interface.encodeFunctionData("setBLSKeyForWallet", [
addressSignature,
wallet2.PublicKey(),
signatureExpiryTimestamp,
]),
};

View File

@@ -37,7 +37,7 @@
"assert-browserify": "^2.0.0",
"async-mutex": "^0.3.2",
"axios": "^0.27.2",
"bls-wallet-clients": "0.9.0-2a20bfe",
"bls-wallet-clients": "0.9.0-3251dec",
"browser-passworder": "^2.0.3",
"bs58check": "^2.1.2",
"crypto-browserify": "^3.12.0",

View File

@@ -1791,16 +1791,6 @@
version "5.7.0"
resolved "https://registry.yarnpkg.com/@ethersproject/transactions/-/transactions-5.7.0.tgz#91318fc24063e057885a6af13fdb703e1f993d3b"
integrity sha512-kmcNicCp1lp8qanMTC3RIikGgoJ80ztTyvtsFvCYpSCfkjhD0jZ2LOrnbcuxuToLIUYYf+4XwD1rP+B/erDIhQ==
dependencies:
"@ethersproject/address" "^5.7.0"
"@ethersproject/bignumber" "^5.7.0"
"@ethersproject/bytes" "^5.7.0"
"@ethersproject/constants" "^5.7.0"
"@ethersproject/keccak256" "^5.7.0"
"@ethersproject/logger" "^5.7.0"
"@ethersproject/properties" "^5.7.0"
"@ethersproject/rlp" "^5.7.0"
"@ethersproject/signing-key" "^5.7.0"
"@ethersproject/transactions@^5.5.0", "@ethersproject/transactions@^5.6.2":
version "5.6.2"
@@ -2898,10 +2888,10 @@ blakejs@^1.1.0:
resolved "https://registry.yarnpkg.com/blakejs/-/blakejs-1.2.1.tgz#5057e4206eadb4a97f7c0b6e197a505042fc3814"
integrity sha512-QXUSXI3QVc/gJME0dBpXrag1kbzOqCjCX8/b54ntNyW6sjtoqxqRk3LTmXzaJoh71zMsDCjM+47jS7XiwN/+fQ==
bls-wallet-clients@0.9.0-2a20bfe:
version "0.9.0-2a20bfe"
resolved "https://registry.yarnpkg.com/bls-wallet-clients/-/bls-wallet-clients-0.9.0-2a20bfe.tgz#2e39757a18df3ba78d816ae15f6b88000443a2a6"
integrity sha512-w4efcArPBEowrAkIdVYc2mOLlkN8E5O9eIqEhoo6IrRVrN21p/JVNdoot4N3o5MAKFbeaYfid/u9lL6p2DNdiw==
bls-wallet-clients@0.9.0-3251dec:
version "0.9.0-3251dec"
resolved "https://registry.npmjs.org/bls-wallet-clients/-/bls-wallet-clients-0.9.0-3251dec.tgz#46f6795110abd90a242fec5a59af1ae2a9f45cd9"
integrity sha512-GBjJToEK3nEkldnWB8971BlIKCF0AT78GsrnkXfC1WtAYsvtqyZxmfGABMmN4si8GgykyyjYcxTypORz61AGZQ==
dependencies:
"@thehubbleproject/bls" "^0.5.1"
ethers "^5.7.2"