From 66948a7269a7aa4582f96b978d4a21f362fc0fc1 Mon Sep 17 00:00:00 2001 From: jordan Date: Fri, 8 Mar 2024 21:31:09 -0500 Subject: [PATCH 1/8] added test to check for bounty using another pauser payer --- test/HoneyPause.t.sol | 56 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 5fbef03..9eceff6 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -542,6 +542,60 @@ contract HoneyPauseTest is Test { fc.setFnRevertData(fnSelector, alwaysRevertData); return payable(fc); } + + function test_BountyCannotUseAnotherBountyPauserPayer() external { + MockPauser mockPauser = new MockPauser(); + MockPayer mockPayer = new MockPayer(); + + testToken.mint(address(mockPayer), 100 ether); + + // Create a legitimate bounty with the mock contracts + uint256 bountyAmount = 1 ether; + uint256 legitimateBountyId = honey.add("LegitimateExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + + // Set the bountyId in mock contracts to the valid bountyId + mockPauser.setValidBountyId(legitimateBountyId); + mockPayer.setValidBountyId(legitimateBountyId); + + // claim the legitimate bounty + IExploiter exploiter = new TestExploiter(); + honey.claim(legitimateBountyId, payable(address(this)), exploiter, "", ""); + + // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifyer + uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + + // Expecting failure due to bountyId checks in the impl of IPauser IPayer + vm.expectRevert("Unauthorized bountyId"); + honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); + } + +} + +contract MockPauser is IPauser { + event PauseCalled(uint256 bountyId); + uint256 private validBountyId; + + function setValidBountyId(uint256 _validBountyId) external { + validBountyId = _validBountyId; + } + + function pause(uint256 bountyId) external { + emit PauseCalled(bountyId); + require(bountyId == validBountyId, "Unauthorized bountyId"); + } +} + +contract MockPayer is IPayer { + uint256 private validBountyId; + + function setValidBountyId(uint256 _validBountyId) external { + validBountyId = _validBountyId; + } + + function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external override { + require(bountyId == validBountyId, "Unauthorized bountyId"); + require(token.transfer(to, amount), "Transfer failed"); + } } contract TestExploiter is IExploiter { @@ -673,4 +727,4 @@ contract TestHoneyPause is HoneyPause { ) external pure { _validateBountyConfig({ operator: operator, pauser: pauser, verifier: verifier, payer: payer }); } -} \ No newline at end of file +} From cc047a2aaeb12f5570335e912493fb771a9c0ada Mon Sep 17 00:00:00 2001 From: JordanCason Date: Sat, 9 Mar 2024 09:10:00 -0500 Subject: [PATCH 2/8] Update test/HoneyPause.t.sol Co-authored-by: Lawrence Forman --- test/HoneyPause.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 9eceff6..5fee128 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -594,7 +594,7 @@ contract MockPayer is IPayer { function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external override { require(bountyId == validBountyId, "Unauthorized bountyId"); - require(token.transfer(to, amount), "Transfer failed"); + token.transfer(to, amount); } } From 1efee104fed8a72111398b24fa5c48c05fd52230 Mon Sep 17 00:00:00 2001 From: JordanCason Date: Sat, 9 Mar 2024 09:10:22 -0500 Subject: [PATCH 3/8] Update test/HoneyPause.t.sol Co-authored-by: Lawrence Forman --- test/HoneyPause.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 5fee128..b39f5e7 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -588,8 +588,8 @@ contract MockPauser is IPauser { contract MockPayer is IPayer { uint256 private validBountyId; - function setValidBountyId(uint256 _validBountyId) external { - validBountyId = _validBountyId; + function setValidBountyId(uint256 validBountyId_) external { + validBountyId = validBountyId_; } function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external override { From 24aaece6a2bbfeec0431cbb7ecc2d97295f5835f Mon Sep 17 00:00:00 2001 From: jordan Date: Sat, 9 Mar 2024 09:27:56 -0500 Subject: [PATCH 4/8] fix some small nits --- test/HoneyPause.t.sol | 58 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index b39f5e7..3c74087 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -499,6 +499,33 @@ contract HoneyPauseTest is Test { assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); } + function test_bountyCannotUseAnotherBountyPauserPayer() external { + MockPauser mockPauser = new MockPauser(); + MockPayer mockPayer = new MockPayer(); + + testToken.mint(address(mockPayer), 100 ether); + + // Create a legitimate bounty with the mock contracts + uint256 bountyAmount = 1 ether; + uint256 legitimateBountyId = honey.add("LegitimateExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + + // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifyer + uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + + // Set the bountyId in mock contracts to the valid bountyId + mockPauser.setValidBountyId(legitimateBountyId); + mockPayer.setValidBountyId(legitimateBountyId); + + IExploiter exploiter = new TestExploiter(); + + // Expecting failure due to bountyId checks in the impl of IPauser IPayer + vm.expectRevert("Unauthorized bountyId"); + honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); + + // claim the legitimate bounty + honey.claim(legitimateBountyId, payable(address(this)), exploiter, "", ""); + } + function _addTestBounty() private returns (uint256 bountyId) { @@ -542,45 +569,16 @@ contract HoneyPauseTest is Test { fc.setFnRevertData(fnSelector, alwaysRevertData); return payable(fc); } - - function test_BountyCannotUseAnotherBountyPauserPayer() external { - MockPauser mockPauser = new MockPauser(); - MockPayer mockPayer = new MockPayer(); - - testToken.mint(address(mockPayer), 100 ether); - - // Create a legitimate bounty with the mock contracts - uint256 bountyAmount = 1 ether; - uint256 legitimateBountyId = honey.add("LegitimateExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); - - // Set the bountyId in mock contracts to the valid bountyId - mockPauser.setValidBountyId(legitimateBountyId); - mockPayer.setValidBountyId(legitimateBountyId); - - // claim the legitimate bounty - IExploiter exploiter = new TestExploiter(); - honey.claim(legitimateBountyId, payable(address(this)), exploiter, "", ""); - - // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifyer - uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); - - // Expecting failure due to bountyId checks in the impl of IPauser IPayer - vm.expectRevert("Unauthorized bountyId"); - honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); - } - } contract MockPauser is IPauser { - event PauseCalled(uint256 bountyId); uint256 private validBountyId; function setValidBountyId(uint256 _validBountyId) external { validBountyId = _validBountyId; } - function pause(uint256 bountyId) external { - emit PauseCalled(bountyId); + function pause(uint256 bountyId) external view { require(bountyId == validBountyId, "Unauthorized bountyId"); } } From b1646ece53b13e9ee453d4a809a864df4aa2c49a Mon Sep 17 00:00:00 2001 From: jordan Date: Sat, 9 Mar 2024 09:43:53 -0500 Subject: [PATCH 5/8] quick nit fix --- test/HoneyPause.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 3c74087..b3cce92 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -512,12 +512,12 @@ contract HoneyPauseTest is Test { // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifyer uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + IExploiter exploiter = new TestExploiter(); + // Set the bountyId in mock contracts to the valid bountyId mockPauser.setValidBountyId(legitimateBountyId); mockPayer.setValidBountyId(legitimateBountyId); - IExploiter exploiter = new TestExploiter(); - // Expecting failure due to bountyId checks in the impl of IPauser IPayer vm.expectRevert("Unauthorized bountyId"); honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); @@ -574,8 +574,8 @@ contract HoneyPauseTest is Test { contract MockPauser is IPauser { uint256 private validBountyId; - function setValidBountyId(uint256 _validBountyId) external { - validBountyId = _validBountyId; + function setValidBountyId(uint256 validBountyId_) external { + validBountyId = validBountyId_; } function pause(uint256 bountyId) external view { From 43bd9f095feacec2b143ae39e9384610e51f40d4 Mon Sep 17 00:00:00 2001 From: JordanCason Date: Sat, 9 Mar 2024 11:03:53 -0500 Subject: [PATCH 6/8] Update test/HoneyPause.t.sol Co-authored-by: Lawrence Forman --- test/HoneyPause.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index b3cce92..45ebe19 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -509,7 +509,7 @@ contract HoneyPauseTest is Test { uint256 bountyAmount = 1 ether; uint256 legitimateBountyId = honey.add("LegitimateExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); - // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifyer + // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifier uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); IExploiter exploiter = new TestExploiter(); From 343a7392b4d7f630557c7a4bf8ce9fd2dc24275d Mon Sep 17 00:00:00 2001 From: jordan Date: Sat, 9 Mar 2024 12:45:24 -0500 Subject: [PATCH 7/8] fixed conflict --- test/HoneyPause.t.sol | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 45ebe19..bb7b289 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -504,26 +504,45 @@ contract HoneyPauseTest is Test { MockPayer mockPayer = new MockPayer(); testToken.mint(address(mockPayer), 100 ether); - - // Create a legitimate bounty with the mock contracts uint256 bountyAmount = 1 ether; - uint256 legitimateBountyId = honey.add("LegitimateExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); - // Create a fake bounty attempting to use the first bounty's pauser/payer with an always successful Verifier - uint256 fakeBountyId = honey.add("BogusExploitTest", testToken, bountyAmount, new TestVerifier(), mockPauser, mockPayer, address(this)); + // Create legitimate and a fake bounty with the mock contracts. + uint256 legitimateBountyId = honey.add( + "Legitimate", + testToken, + bountyAmount, + new TestVerifier(), + mockPauser, + mockPayer, + address(this) + ); + + uint256 fakeBountyId = honey.add( + "ExploitTest", + testToken, + bountyAmount, + new TestVerifier(), + mockPauser, + mockPayer, + address(this) + ); IExploiter exploiter = new TestExploiter(); - // Set the bountyId in mock contracts to the valid bountyId + // Set the valid bountyId in mock contracts to the legitimate bountyId. mockPauser.setValidBountyId(legitimateBountyId); - mockPayer.setValidBountyId(legitimateBountyId); + mockPayer.setValidBountyId(legitimateBountyId); - // Expecting failure due to bountyId checks in the impl of IPauser IPayer + // Test that the pauser receives the correct bountyId and reverts against the fake bountyId. vm.expectRevert("Unauthorized bountyId"); honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); - // claim the legitimate bounty - honey.claim(legitimateBountyId, payable(address(this)), exploiter, "", ""); + // Temporarily allow pausing for the fake bounty to test payer logic. + mockPauser.setValidBountyId(fakeBountyId); + + // Test that the payer also correctly identifies and reverts against fake bountyId claims. + vm.expectRevert("Unauthorized bountyId"); + honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); } function _addTestBounty() From 9e3d5d86561a1b7f79d2c5ce6007bf4b7c7c4459 Mon Sep 17 00:00:00 2001 From: jordan Date: Sat, 9 Mar 2024 13:13:00 -0500 Subject: [PATCH 8/8] updated revert messages --- test/HoneyPause.t.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index bb7b289..fda060b 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -503,7 +503,6 @@ contract HoneyPauseTest is Test { MockPauser mockPauser = new MockPauser(); MockPayer mockPayer = new MockPayer(); - testToken.mint(address(mockPayer), 100 ether); uint256 bountyAmount = 1 ether; // Create legitimate and a fake bounty with the mock contracts. @@ -534,14 +533,14 @@ contract HoneyPauseTest is Test { mockPayer.setValidBountyId(legitimateBountyId); // Test that the pauser receives the correct bountyId and reverts against the fake bountyId. - vm.expectRevert("Unauthorized bountyId"); + vm.expectRevert("Unauthorized bountyId in pauser"); honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); // Temporarily allow pausing for the fake bounty to test payer logic. mockPauser.setValidBountyId(fakeBountyId); // Test that the payer also correctly identifies and reverts against fake bountyId claims. - vm.expectRevert("Unauthorized bountyId"); + vm.expectRevert("Unauthorized bountyId in payer"); honey.claim(fakeBountyId, payable(address(this)), exploiter, "", ""); } @@ -598,7 +597,7 @@ contract MockPauser is IPauser { } function pause(uint256 bountyId) external view { - require(bountyId == validBountyId, "Unauthorized bountyId"); + require(bountyId == validBountyId, "Unauthorized bountyId in pauser"); } } @@ -610,7 +609,7 @@ contract MockPayer is IPayer { } function payExploiter(uint256 bountyId, ERC20 token, address payable to, uint256 amount) external override { - require(bountyId == validBountyId, "Unauthorized bountyId"); + require(bountyId == validBountyId, "Unauthorized bountyId in payer"); token.transfer(to, amount); } }