TRUST H-3: calling incorrect hasPermission() function (#1003)

reserve-protocol · Oct 31, 2023 · 6318c68 · 6318c68
1 parent 6c7e449
commit 6318c68
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 2 deletions.
diff --git a/contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol b/contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol
@@ -81,7 +81,7 @@ contract CusdcV3Wrapper is ICusdcV3Wrapper, WrappedERC20, CometHelpers {
         address dst,
         uint256 amount
     ) internal {
-        if (!hasPermission(src, operator)) revert Unauthorized();
+        if (!underlyingComet.hasPermission(src, operator)) revert Unauthorized();
         // {Comet}
         uint256 srcBal = underlyingComet.balanceOf(src);
         if (amount > srcBal) amount = srcBal;

diff --git a/contracts/plugins/assets/compoundv3/vendor/CometExtInterface.sol b/contracts/plugins/assets/compoundv3/vendor/CometExtInterface.sol
@@ -95,4 +95,12 @@ abstract contract CometExtInterface {
     function allowance(address owner, address spender) external view virtual returns (uint256);
 
     event Approval(address indexed owner, address indexed spender, uint256 amount);
+
+    /**
+     * @notice Determine if the manager has permission to act on behalf of the owner
+     * @param owner The owner account
+     * @param manager The manager account
+     * @return Whether or not the manager has permission
+     */
+    function hasPermission(address owner, address manager) external view virtual returns (bool);
 }
diff --git a/test/plugins/individual-collateral/compoundv3/CusdcV3Wrapper.test.ts b/test/plugins/individual-collateral/compoundv3/CusdcV3Wrapper.test.ts
@@ -103,12 +103,43 @@ describeFork('Wrapped CUSDCv3', () => {
       expect(await wcusdcV3.balanceOf(don.address)).to.eq(expectedAmount)
     })
 
+    it('checks for correct approval on deposit - regression test', async () => {
+      await expect(
+        wcusdcV3.connect(don).depositFrom(bob.address, charles.address, ethers.constants.MaxUint256)
+      ).revertedWithCustomError(wcusdcV3, 'Unauthorized')
+
+      // Provide approval on the wrapper
+      await wcusdcV3.connect(bob).allow(don.address, true)
+
+      const expectedAmount = await wcusdcV3.convertDynamicToStatic(
+        await cusdcV3.balanceOf(bob.address)
+      )
+
+      // This should fail even when bob approved wcusdcv3 to spend his tokens,
+      // because there is no explicit approval of cUSDCv3 from bob to don, only
+      // approval on the wrapper
+      await expect(
+        wcusdcV3.connect(don).depositFrom(bob.address, charles.address, ethers.constants.MaxUint256)
+      ).to.be.revertedWithCustomError(cusdcV3, 'Unauthorized')
+
+      // Add explicit approval of cUSDCv3 and retry
+      await cusdcV3.connect(bob).allow(don.address, true)
+      await wcusdcV3
+        .connect(don)
+        .depositFrom(bob.address, charles.address, ethers.constants.MaxUint256)
+
+      expect(await wcusdcV3.balanceOf(bob.address)).to.eq(0)
+      expect(await wcusdcV3.balanceOf(charles.address)).to.eq(expectedAmount)
+    })
+
     it('deposits from a different account', async () => {
       expect(await wcusdcV3.balanceOf(charles.address)).to.eq(0)
       await expect(
         wcusdcV3.connect(don).depositFrom(bob.address, charles.address, ethers.constants.MaxUint256)
       ).revertedWithCustomError(wcusdcV3, 'Unauthorized')
-      await wcusdcV3.connect(bob).connect(bob).allow(don.address, true)
+
+      // Approval has to be on cUsdcV3, not the wrapper
+      await cusdcV3.connect(bob).allow(don.address, true)
       const expectedAmount = await wcusdcV3.convertDynamicToStatic(
         await cusdcV3.balanceOf(bob.address)
       )