Merge pull request #179 from dcrescimbeni/fix/dos-on-withdrawalTokens

Fix/DOS on withdrawal tokens
DXgovernance · Jun 21, 2022 · c284fc7 · c284fc7
2 parents ac3eca0 + be1532a
commit c284fc7
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 0 deletions.
diff --git a/contracts/erc20guild/BaseERC20Guild.sol b/contracts/erc20guild/BaseERC20Guild.sol
@@ -438,6 +438,7 @@ contract BaseERC20Guild {
     function withdrawTokens(uint256 tokenAmount) external virtual {
         require(votingPowerOf(msg.sender) >= tokenAmount, "ERC20Guild: Unable to withdraw more tokens than locked");
         require(tokensLocked[msg.sender].timestamp < block.timestamp, "ERC20Guild: Tokens still locked");
+        require(tokenAmount > 0, "ERC20Guild: amount of tokens to withdraw must be greater than 0");
         tokensLocked[msg.sender].amount = tokensLocked[msg.sender].amount.sub(tokenAmount);
         totalLocked = totalLocked.sub(tokenAmount);
         tokenVault.withdraw(msg.sender, tokenAmount);

diff --git a/contracts/erc20guild/implementations/SnapshotERC20Guild.sol b/contracts/erc20guild/implementations/SnapshotERC20Guild.sol
@@ -98,6 +98,7 @@ contract SnapshotERC20Guild is ERC20GuildUpgradeable {
             "SnapshotERC20Guild: Unable to withdraw more tokens than locked"
         );
         require(tokensLocked[msg.sender].timestamp < block.timestamp, "SnapshotERC20Guild: Tokens still locked");
+        require(tokenAmount > 0, "SnapshotERC20Guild: amount of tokens to withdraw must be greater than 0");
         _updateAccountSnapshot(msg.sender);
         _updateTotalSupplySnapshot();
         tokensLocked[msg.sender].amount = tokensLocked[msg.sender].amount.sub(tokenAmount);

diff --git a/test/erc20guild/ERC20Guild.js b/test/erc20guild/ERC20Guild.js
@@ -1296,6 +1296,11 @@ contract("ERC20Guild", function (accounts) {
         "ERC20: transfer amount exceeds balance"
       );
 
+      // Cannot withdraw zero tokens
+      await expectRevert(
+        erc20Guild.withdrawTokens(0, { from: accounts[1] }),
+        "ERC20Guild: amount of tokens to withdraw must be greater than 0"
+      );
       // Cant lock zero tokens
       await expectRevert(
         erc20Guild.lockTokens(0, { from: accounts[1] }),

diff --git a/test/erc20guild/implementations/SnapshotERC2Guild.js b/test/erc20guild/implementations/SnapshotERC2Guild.js
@@ -204,6 +204,12 @@ contract("SnapshotERC20Guild", function (accounts) {
         "ERC20: transfer amount exceeds balance"
       );
 
+      // Cannot withdraw zero tokens
+      await expectRevert(
+        erc20Guild.withdrawTokens(0, { from: accounts[1] }),
+        "SnapshotERC20Guild: amount of tokens to withdraw must be greater than 0"
+      );
+
       // try to release more than locked and fail
       await expectRevert(
         erc20Guild.withdrawTokens(50001, { from: accounts[1] }),