From 6318c6850a8e9480c33f39af1a02ac63fb5fedb6 Mon Sep 17 00:00:00 2001 From: "Julian M. Rodriguez" <56316686+julianmrodri@users.noreply.github.com> Date: Tue, 31 Oct 2023 14:18:54 -0300 Subject: [PATCH] TRUST H-3: calling incorrect hasPermission() function (#1003) --- .../assets/compoundv3/CusdcV3Wrapper.sol | 2 +- .../compoundv3/vendor/CometExtInterface.sol | 8 +++++ .../compoundv3/CusdcV3Wrapper.test.ts | 33 ++++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol b/contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol index 69791aac72..3706d9d339 100644 --- 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 index a144d69112..70d9664aac 100644 --- 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 index b47b71bff9..7a1babf104 100644 --- 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) )