We were recently approached by 0xkasper from hexens, a white hat security team, who found a new attack vector based on a vulnerability we were aware of in the Nouns token contract. Nouns tech grants pod has initiated a bounty payment of 30K USDC to the team.
Who’s at risk? Noun owners delegating to EOA accounts (not smart contract accounts).
What’s the damage? Noun NFTs can become non-transferrable with no voting power.
How likely is it to happen? The probability is low if you trust your delegates and their wallet security setup; otherwise please consider changing your delegation immediately.
Alice owns Noun 1, and Bob owns Noun 2.Alice delegates to Bob (Alice votes: 0; Bob votes: 2).
The problem begins when Bob, whether intentionally or not, uses a faulty delegation function.
Bob uses the delegateBySig function in the Nouns token contract to delegate to address zero (0x0000…). By doing so, one voting power unit was burned and can never be recovered (Alice votes: 0; Bob votes: 1). This is because most of the code does not expect delegation to address zero to be possible, but delegateBySig allows such a delegation to pass into the voting power change code.
Continuing the story:
Bob transfers Noun 2 to a new wallet, let’s name it Bobby (Alice votes: 0; Bob votes: 0; Bobby votes: 1).
Now Alice's Noun 1 is a zombie:
its voting power is gone forever.
It can never be transferred out of Alice's wallet (a transfer attempt will always revert).
The reason why transfer attempts will revert is because upon each transfer the Nouns contract reduces the sender’s voting power by one, and since Bob is already at zero, that is an invalid operation (an underflow error).
The conditions under which this can happen:
You delegate to an EOA account (only EOA accounts can use delegateBySig because it requires a signature with a private key).
That EOA account has at least one Noun that it owns.
For more technical information see the last section of this post.
The obvious case to worry about is delegating to people you do not trust, who might want to grief you. However, even if you trust your delegate’s intention, they might be subjected to a phishing attack or some other hack, and their attacker might perform these actions to grief you.
Unfortunately, the NounsToken contract is not upgradeable, so we cannot fix the bug directly, instead we must work around it. The fact that smart contract delegate accounts cannot participate in this attack is very useful.
For now, the safest bet is to delegate to smart contract accounts only. Ask your delegates to create a Gnosis Safe account and delegate to them there. Until then if you are not sure what to do, delegate back to your Noun owner account.
For the long term, there are a few design options we’re exploring for how to mitigate this attack vector with a less cumbersome user experience. We will share more information in an upcoming post.
That’s all for the high level summary. You can find additional technical information below.
Thanks,verbs ⌐◨-◨
For those of you who made it this far and care to know exactly how this attack works, here’s a deeper dive.
The main function to understand is _moveDelegates which is called every time a noun is transferred or when a noun owner account delegates to any other account. See the function code below.
Let’s follow the story above from a more technical point of view.
When a noun is bought on auction, it is first minted to the auction house. Minting looks like a transfer from address zero to the recipient. If you follow the code you’ll notice that there’s a special condition regarding address zero that skips the voting power update code for any source or recipient that is address zero. So when a noun is minted, a new voting power unit is created and added to the auction house’s voting power balance (this happens in the _writeCheckpoint function).
When Alice and Bob bought their respective nouns, the _moveDelegates function ran again, this time deducting one vote from the auction house, and incrementing the buyer’s voting power by one vote.
When Alice delegated to Bob, this function ran again, this time writing a checkpoint for Alice saying she has zero votes, while giving Bob one more vote, for a total of two (srcRep would be Alice, dstRep would be Bob, and amount would be one, equal to the voting power Alice owns).
When Bob used delegateBySig and provided the zero address as the delegate, srcRep was Bob, and dstRep was address(0). Notice how the first part of _moveDelegates that updates srcRep votes executed, while the second part that updates dstRep votes did not execute because dstRep was address(0) – this is the key moment in the attack vector. The result of this transaction is that a vote was deducted, never to be added again. The total number of votes represented in all the latest checkpoints combined are now one short of the Nouns NFT total supply.
At this moment in time it’s not obvious that Alice should suffer from the situation. In theory, if Bob does nothing else, Alice can still pull her delegation from Bob and her noun wouldn’t become a zombie; in that case it would be Bob’s noun that becomes a zombie.
The crux of the griefing is when Bob immediately after delegating to address zero, also transfers his noun to a different wallet; in this action _moveDelegates runs again, decreasing Bob’s vote count from one to zero, and increasing Bob’s new wallet’s votes to one. This is another side of the bug; in valid delegation scenarios when Bob delegates to Charlie and Bob transfers a noun they own, a vote is deducted from Charlie’s votes, since that is in fact where Bob’s noun’s vote resides. In our address zero buggy scenario, due to how the view function delegates is implemented, address zero is a magic value that assumes an account is its own delegate, so from the code’s point of view Bob is his own delegate, not address zero, and so Bob’s vote count is decreased.
Remember, Alice is still delegating to Bob, while Bob has zero votes. If she tries to undelegate from Bob, _moveDelegates is called where srcRep is Bob and dstRep is Alice, and the line that calculates srcRepNew will revert with the underflow error you can see in the code, because it would be attempt to run the calculation 0 - 1 on an unsigned integer.
The same problem arises if Alice tries to transfer her noun to a different account. The function _beforeTokenTransfer is executed, and in it again _moveDelegates is called, moving votes from Alice’s delegate, Bob, to the recipient’s delegate, which by default is the recipient account itself. And so again the code is run with Bob as srcRep, and we hit the underflow error.
At this moment again, it’s not entirely the case that Alice’s noun has to get stuck in her wallet with no voting power. If for whatever reason Bob buys another noun, or any other noun owner delegates votes to Bob, Bob’s votes go above zero again, and Alice then has the opportunity to undelegate or transfer her noun elsewhere. The problem of the zombie noun would then transfer to whichever noun remains owned by or delegated to Bob.
function _moveDelegates(
address srcRep,
address dstRep,
uint96 amount
) internal {
if (srcRep != dstRep && amount > 0) {
if (srcRep != address(0)) {
uint32 srcRepNum = numCheckpoints[srcRep];
uint96 srcRepOld = srcRepNum > 0 ? checkpoints[srcRep][srcRepNum - 1].votes : 0;
uint96 srcRepNew = sub96(srcRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount underflows');
_writeCheckpoint(srcRep, srcRepNum, srcRepOld, srcRepNew);
}
if (dstRep != address(0)) {
uint32 dstRepNum = numCheckpoints[dstRep];
uint96 dstRepOld = dstRepNum > 0 ? checkpoints[dstRep][dstRepNum - 1].votes : 0;
uint96 dstRepNew = add96(dstRepOld, amount, 'ERC721Checkpointable::_moveDelegates: amount overflows');
_writeCheckpoint(dstRep, dstRepNum, dstRepOld, dstRepNew);
}
}
}
function _writeCheckpoint(
address delegatee,
uint32 nCheckpoints,
uint96 oldVotes,
uint96 newVotes
) internal {
uint32 blockNumber = safe32(
block.number,
'ERC721Checkpointable::_writeCheckpoint: block number exceeds 32 bits'
);
if (nCheckpoints > 0 && checkpoints[delegatee][nCheckpoints - 1].fromBlock == blockNumber) {
checkpoints[delegatee][nCheckpoints - 1].votes = newVotes;
} else {
checkpoints[delegatee][nCheckpoints] = Checkpoint(blockNumber, newVotes);
numCheckpoints[delegatee] = nCheckpoints + 1;
}
emit DelegateVotesChanged(delegatee, oldVotes, newVotes);
}
function _beforeTokenTransfer(
address from,
address to,
uint256 tokenId
) internal override {
super._beforeTokenTransfer(from, to, tokenId);
/// @notice Differs from `_transferTokens()` to use `delegates` override method to simulate auto-delegation
_moveDelegates(delegates(from), delegates(to), 1);
}
function delegates(address delegator) public view returns (address) {
address current = _delegates[delegator];
return current == address(0) ? delegator : current;
}