[N01] Lack of input validation (#6989)

celo-org · Feb 16, 2021 · c0faae7 · c0faae7
1 parent 22267c6
commit c0faae7
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 2 deletions.
diff --git a/packages/protocol/contracts/stability/Reserve.sol b/packages/protocol/contracts/stability/Reserve.sol
@@ -298,6 +298,7 @@ contract Reserve is
    * @param spender The address that is allowed to spend Reserve funds.
    */
   function addSpender(address spender) external onlyOwner {
+    require(address(0) != spender, "Spender can't be null");
     isSpender[spender] = true;
     emit SpenderAdded(spender);
   }
@@ -325,6 +326,7 @@ contract Reserve is
    * @param spender The address that is allowed to spend Reserve funds.
    */
   function addExchangeSpender(address spender) external onlyOwner {
+    require(address(0) != spender, "Spender can't be null");
     require(!isExchangeSpender[spender], "Address is already Exchange Spender");
     isExchangeSpender[spender] = true;
     exchangeSpenderAddresses.push(spender);

diff --git a/packages/protocol/test/stability/reserve.ts b/packages/protocol/test/stability/reserve.ts
@@ -303,6 +303,10 @@ contract('Reserve', (accounts: string[]) => {
       })
     })
 
+    it('does not allow an empty address', async () => {
+      await assertRevert(reserve.addExchangeSpender('0x0000000000000000000000000000000000000000'))
+    })
+
     it('has the right list of exchange spenders after addition', async () => {
       await reserve.addExchangeSpender(exchangeAddress)
       await reserve.addExchangeSpender(accounts[1])
@@ -373,13 +377,17 @@ contract('Reserve', (accounts: string[]) => {
     it('emits on add', async () => {
       const addSpenderTx = await reserve.addSpender(spender)
 
-      const addExchangeSpenderTxLogs = addSpenderTx.logs.filter((x) => x.event === 'SpenderAdded')
-      assert(addExchangeSpenderTxLogs.length === 1, 'Did not receive event')
+      const addSpenderTxLogs = addSpenderTx.logs.filter((x) => x.event === 'SpenderAdded')
+      assert(addSpenderTxLogs.length === 1, 'Did not receive event')
     })
 
     it('only allows owner', async () => {
       await assertRevert(reserve.addSpender(nonOwner, { from: nonOwner }))
     })
+
+    it('does not allow an empty address', async () => {
+      await assertRevert(reserve.addSpender('0x0000000000000000000000000000000000000000'))
+    })
   })
 
   describe('#removeSpender(spender)', () => {