NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cause _addSigners to revert if it triggers a totalWeight overflow (#5790)

Browse Source
This commit is contained in:
Hadrien Croubois
2025-07-09 09:08:13 +02:00
committed by GitHub
parent f9f7db03f3
commit ba35d580f4
2 changed files with 30 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
contracts/utils/cryptography/signers/MultiSignerERC7913Weighted.sol
View File

@ -129,6 +129,20 @@ abstract contract MultiSignerERC7913Weighted is MultiSignerERC7913 {
_validateReachableThreshold();
}
/**
* @dev See {MultiSignerERC7913-_addSigners}.
*
* In cases where {totalWeight} is almost `type(uint64).max` (due to a large `_totalExtraWeight`), adding new
* signers could cause the {totalWeight} computation to overflow. Adding a {totalWeight} calls after the new
* signers are added ensures no such overflow happens.
*/
function _addSigners(bytes[] memory newSigners) internal virtual override {
super._addSigners(newSigners);
// This will revert if the new signers cause an overflow
_validateReachableThreshold();
}
/**
* @dev See {MultiSignerERC7913-_removeSigners}.
*

16
test/account/AccountMultiSignerWeighted.test.js
View File

@ -6,6 +6,7 @@ const { getDomain } = require('../helpers/eip712');
const { ERC4337Helper } = require('../helpers/erc4337');
const { NonNativeSigner, P256SigningKey, RSASHA256SigningKey, MultiERC7913SigningKey } = require('../helpers/signers');
const { PackedUserOperation } = require('../helpers/eip712-types');
const { MAX_UINT64 } = require('../helpers/constants');
const { shouldBehaveLikeAccountCore, shouldBehaveLikeAccountHolder } = require('./Account.behavior');
const { shouldBehaveLikeERC1271 } = require('../utils/cryptography/ERC1271.behavior');
@ -292,5 +293,20 @@ describe('AccountMultiSignerWeighted', function () {
.withArgs(signer1)
.to.not.emit(this.mock, 'ERC7913SignerWeightChanged');
});
it('should revert if total weight to overflow (_setSignerWeights)', async function () {
await expect(this.mock.$_setSignerWeights([signer1, signer2, signer3], [1n, 1n, MAX_UINT64 - 1n]))
.to.be.revertedWithCustomError(this.mock, 'SafeCastOverflowedUintDowncast')
.withArgs(64, MAX_UINT64 + 1n);
});
it('should revert if total weight to overflow (_addSigner)', async function () {
await this.mock.$_setSignerWeights([signer1, signer2, signer3], [1n, 1n, MAX_UINT64 - 2n]);
await expect(this.mock.totalWeight()).to.eventually.equal(MAX_UINT64);
await expect(this.mock.$_addSigners([signer4]))
.to.be.revertedWithCustomError(this.mock, 'SafeCastOverflowedUintDowncast')
.withArgs(64, MAX_UINT64 + 1n);
});
});
});