rvierdiiev - Courier can be cheated to avoid fees

[sherlock-admin2]

rvierdiiev

medium

Courier can be cheated to avoid fees

Summary

User can avoid paying fees to courier by providing his address.

Vulnerability Detail

Courier is the entity that will get some percentage of user's profit, when user withdraws shares. Why someone will be providing fees to that courier? Because courier can provide very comfortable website for user, so he is pleased to pay for that service.

In case if any frontend will deposit on behalf of user, then they should have at least 1 wei allowance from user. In this case they have ability to set themselves as courier for user.

Courier is set inside _mint function. There is one important thing: in case if user already has shares, then courier will not be set, which means that whoever did that deposit, he will not receive any fees in the end.

So how user can use this? The most simple way is to frontrun deposit call and transfer some amount of shares to his account from another account. As result, his shares amount will not be 0 anymore and fees will be avoid.

Another problem is that when user has used one website and then switched to another one, then new service expects that user will pay fees for using it, but in reality after new deposit, only old frontend will receive all fees.

Impact

User can cheat couriers.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

I guess that deposit function should check if provided courier will be set or not and early revert if not.

[sherlock-admin2]

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

panprog commented:

borderline low/medium, it's indeed possible to deposit 1 wei frontrunning deposit with courier, but there is no profit in it for the attacker, dup of #37

MohammedRizwan commented:

invalid issue

[haydenshively]

Probability of exploit is low, but we'll try to improve the courier flows referenced here and in #37, #39, #80, etc.

[Shogoki]

Escalate This is a low severity issue. Only thing that is missed on is affiliate fees. IMHO It is like using an affiliate link. The user can decide to use it or not. Also, it is not necessary, in case of a frontend provider to get an allowance first. The frontend could simply set itself as a courier on the deposit transaction, which gets executed in the users context.

[sherlock-admin2]

Escalate This is a low severity issue. Only thing that is missed on is affiliate fees. IMHO It is like using an affiliate link. The user can decide to use it or not. Also, it is not necessary, in case of a frontend provider to get an allowance first. The frontend could simply set itself as a courier on the deposit transaction, which gets executed in the users context.

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.

[roguereddwarf]

@Shogoki Yes, the user can decide it to use it or not. But if the user decides to use it, nobody should make him not use it.

What remains is that a legitimate actor in the protocol (the courier) can be cheated out of their "affiliate fees".

Since there is this direct loss of "affiliate fees" as a result of a griefing attack (no profit-motive) this is a clear Medium imo.

The need for the provider to have an allowance comes from this line: https://github.com/aloelabs/aloe-ii/blob/c71e7b0cfdec830b1f054486dfe9d58ce407c7a4/core/src/Lender.sol#L121

If the provider doesn't have an allowance for the beneficiary, the transaction reverts.

[Trumpero]

Planning to reject escalation and keep this issue at a medium severity.

When users use an affiliate link, attackers can do a griefing attack to disable the affiliate fee of the courier.

[cvetanovv]

In my opinion, this is a medium severity

[haydenshively]

Fixed in https://github.com/aloelabs/aloe-ii/pull/216

[Czar102]

Result: Medium Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• Shogoki: rejected

[roguereddwarf]

Mitigation Review:

The front-running risk is fixed by implementing a periphery function that first redeems all shares to the user before making the deposit. Thereby the shares balance of the user is zero and the courier can always be set.

Note that in some edge cases when the Lender's balance is insufficient, the redemption may not redeem all shares, meaning the courier will not be set. This is an extreme edge case and is probably negligible.