sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

minhtrng - Cant set principal for Notional #227

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

minhtrng

unlabeled

Cant set principal for Notional

Summary

Incorrect usage of Lender.approve in MarketPlace.setPrincipal causes the latter function to be bricked for setting a Notional principle token.

Vulnerability Detail

The MarketPlace.setPrincipal function calls Lender.approve like this in the case the principal is meant to be from Notional:

else if (p == uint8(Principals.Notional)) {
    ILender(lender).approve(address(0), address(0), address(0), a);

In the corresponding Lender.approve function this happens:

// u is the first param and address of the underlying token to be approved
IERC20 uToken = IERC20(u);
...
if (n != address(0)) {
    Safe.approve(uToken, n, max);
}

As the first parameter passed to approve is address(0), this will attempt to perform a safe approve on the zero address which will revert, making setting the a principle token for notional impossible.

Impact

Not being able to set a principal token after market creation might affect the core purposes of the illuminate iPT by having one principal token less in the basket of integrated principal tokens (at least for a certain underlying and maturity date).

Code Snippet

see #Vulnerability Detail

Tool used

Manual Review

Recommendation

Pass the underlying token address to the approve function as first parameter:

ILender(lender).approve(u, address(0), address(0), a);

Duplicate of #41

Minh-Trng commented 1 year ago

Escalate for 100 USDC Duplicate of #41 I suspect this might have been closed without further checking because I have not provided a link to the corresponding part of the source code. To my own defense, the bot that checked for correct formatting was down during that period and I was not familiar with the conventions as this is my second sherlock contest. The source code that is affected by the issue is still described unambigously.

sherlock-admin commented 1 year ago

Escalate for 100 USDC Duplicate of #41 I suspect this might have been closed without further checking because I have not provided a link to the corresponding part of the source code. To my own defense, the bot that checked for correct formatting was down during that period and I was not familiar with the conventions as this is my second sherlock contest. The source code that is affected by the issue is still described unambigously.

You've created a valid escalation for 100 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

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

Evert0x commented 1 year ago

Escalation accepted

sherlock-admin commented 1 year ago

Escalation accepted

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.