-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix vulnerability that allows bidders to block people who will outbid #4
Fix vulnerability that allows bidders to block people who will outbid #4
Conversation
HOOK-826 Fix vulnerability related to bidders who revert on refunds
Currently, a bidder can prevent themselves from being outbid by reverting on refunds received. |
…vulnerability-related-to-bidders-who
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit. Can discuss later
@@ -0,0 +1,25 @@ | |||
pragma solidity ^0.8.10; | |||
|
|||
import "../../../interfaces/IHookCoveredCall.sol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we start doing direct imports instead of long relative import paths like this?
Maybe if it's a ../
relative is fine. Otherwise direct
What is direct?
*Jake Nyquist*
*CEO @ Hook ( https://hook.xyz )*
Telegram: @jakenyquist
Twitter: @jake_nyquist
…On Mon, May 2 2022 at 2:11 PM, Regynald Augustin < ***@***.*** > wrote:
***@***.**** approved this pull request.
Small nit. Can discuss later
In src/test/utils/mocks/MaliciousBidder.sol (
#4 (comment) ) :
> @@ -0,0 +1,25 @@
+pragma solidity ^0.8.10;
+
+import
"../../../interfaces/IHookCoveredCall.sol";
Should we start doing direct imports instead of long relative import paths
like this?
Maybe if it's a../ relative is fine. Otherwise direct
—
Reply to this email directly, view it on GitHub (
#4 (review) ) ,
or unsubscribe (
https://github.com/notifications/unsubscribe-auth/ACD6HPZBH757VIYMIY4NEZ3VIALFJANCNFSM5UP3JZMQ
).
You are receiving this because you authored the thread. Message ID: <hookart/protocol/pull/4/review/959386403
@ github. com>
|
https://docs.soliditylang.org/en/v0.8.11/path-resolution.html#imports |
When a bidder is outbid, we return the money to them. Currently, they are able to cause the competing bid's call to revert by bidding with a contract that throws when payments are sent.
This fixes that issue and creates a test case to ensure that, even if a bidder attempts to avoid refunds, someone else can outbid them (and settle etc).