sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

hack3r-0m - executing orders might get broken due to console.log #191

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

hack3r-0m

medium

executing orders might get broken due to console.log

Summary

executing orders might get broken due to console.log

Vulnerability Detail

console lib of hardhat calls a specific address 0x000000000000000000636F6e736F6c652e6c6f67 which if has non-zero code then can consume all gas and cause out of gas revert.

this is likely dev enviornment leftover, any untrusted address should not be called in production

Impact

It can lead to contracts being unusable for that specific path of execution (which is "executing orders" in this case) leading to denial-of-service if that un-trusted address has non-zero bytecode

Code Snippet

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/order/IncreaseOrderUtils.sol#L106-L108

Tool used

Manual Review

Recommendation

remove console.log and replace with emitting event if required.

hack3r-0m commented 1 year ago

Escalate for 10 USDC

This should be medium severity under https://docs.sherlock.xyz/audits/judging/judging#how-to-identify-a-medium-issue:

Causes a loss of funds but requires certain external conditions or specific states.

required condition is mentioned in report

sherlock-admin commented 1 year ago

Escalate for 10 USDC

This should be medium severity under https://docs.sherlock.xyz/audits/judging/judging#how-to-identify-a-medium-issue:

Causes a loss of funds but requires certain external conditions or specific states.

required condition is mentioned in report

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

IllIllI000 commented 1 year ago

It's not possible for someone to deploy a contract to this specific address, as that would require being able to create a keccak256() collision. If it were possible, Ethereum security would be broken - Invalid

hrishibhat commented 1 year ago

Escalation rejected

Not a valid high/medium issue as shown in the comment above

sherlock-admin commented 1 year ago

Escalation rejected

Not a valid high/medium issue as shown in the comment above

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.