sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

m4ttm - Reverse order of constructor arguments for ERC20 name and symbol #116

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

m4ttm

medium

Reverse order of constructor arguments for ERC20 name and symbol

Summary

WrappedAvail inherits from OpenZeppelin's ERC20 and passes hardcoded strings to the constructor. These are in reverse order, setting the name to the symbol and vice versa.

Vulnerability Detail

ERC20 takes name first and symbol second, however in the code these are the opposite way around.

Impact

The ERC20 name is set to the intended symbol, and the ERC20 symbol is set to the intended name.

Code Snippet

https://github.com/sherlock-audit/2023-12-avail/blob/1afb56b8d4dfbf5d3f21bed0ddf80a04730204b5/contracts/src/WrappedAvail.sol#L18

constructor(address _bridge) ERC20Permit("Wrapped Avail") ERC20("WAVAIL", "Wrapped Avail") {

Tool used

Manual Review

Recommendation

Change the order of constructor arguments.

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid because {valid: valid medium findings; watson showed a mistyped constructor where the symbol comes first instead of name}

m4ttm commented 8 months ago

Escalate I think this is valid because the token would be deployed with the wrong name and symbol

sherlock-admin commented 8 months ago

Escalate I think this is valid because the token would be deployed with the wrong name and symbol

You've created a valid escalation!

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.

r0ck3tzx commented 8 months ago

This does not justify the medium severity. The incorrect name and symbol will have implications only in regard to off-chain components, which are out-of-scope. Moreover, it will result in the view functions name() and symbol() returning incorrect values, which, by Sherlock's rules, are considered low severity by default:

QEDK commented 8 months ago

This is imo a valid low / QA issue and will be fixed.

cvetanovv commented 8 months ago

I agree with the two comments above. This is valid Low.

Evert0x commented 8 months ago

Result: Unique Invalid

sherlock-admin2 commented 8 months ago

Escalations have been resolved successfully!

Escalation status: