Cashback on referral

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/aaf6c116345f3647e11a35010f28e3b90e7b4862/contracts/core/modules/collect/FeeCollectModule.sol#L99

Vulnerability details

Impact

In the fee collect modules like FeeCollectModule there is no prevention of someone submitting a second profile they own as the referrerProfileId in processCollect to receive back part of the fees paid.

The referral system is essentially broken as all rational agents will submit a second profile they control to get back part of the fees. One could even create a referrer smart contract profile that anyone can submit which automatically refunds the fee received. A similar royalties/referral fees issue was judged high-severity recently.

Recommended Mitigation Steps

There's no way to avoid this except by not allowing any profile as a referrer. Whitelist certain important infrastructure providers, like different frontends, as referrers and only allow these to be used instead of users submitting their alt profiles.

[Zer0dot]

Not sure if this makes sense, using your own referrer as a profile would basically be the equivalent of just having your publication collected directly without referral. In which case, what is the vulnerability?

[donosonaumczuk]

I think they mean the case where Alice posts something and Bob wants to collect it. So Bob instead of just collecting directly from Alice publication, he creates a secondary profile, Bob2, mirrors publication through Bob2 and now Bob collects through Bob2's mirror, basically using the referral fee as a collecting discount for himself.

Even if we enforce referral profile owner to be different than collecting profile owner (aka comparing ownerOf(profileId) instead of profileId) people can just use different addresses for the purpose.

I saw this before but I assumed most of people will use the frontend, which will handle this automatically. But I believe that in the long term is possible that people learn the trick and, as rational agents, use it.

Awaiting for @Zer0dot thoughts on this.

[Zer0dot]

I see, thanks @donosonaumczuk, at the end of the day this is a social protocol. Just like how on legacy social media, users can spam/abuse things, it's a given here that we will never have a fully foolproof system. Plus, with the chain history being visible, tools can (and I'd like to see them) be built to filter out nasty behavior. @oneski want to weigh in?

[donosonaumczuk]

I think there is nothing more to do here

[0xleastwood]

I think its naive to think that this will be properly handled by the front-end and won't be abused by users at some point in time. As such, I'm inclined to treat this as a valid medium risk issue because it fits the category of value leakage by the protocol. However, I can understand that there is no easy fix to this because it is impossible to link EOAs as originating from the same person.

[Zer0dot]

Yeah I think that's fair, since we're dealing with a social protocol there's a degree of social "etiquette" expected from users. Since this does not harm the protocol or the original publication creator, I don't see it as a significant flaw. However, it does of course harm curators. I tend to disagree on the point about frontends @0xleastwood though, it seems to me that if this were to be a problem, it would be fairly straightforward to point fingers at those abusing the system.

[0xleastwood]

Yeah I think that's fair, since we're dealing with a social protocol there's a degree of social "etiquette" expected from users. Since this does not harm the protocol or the original publication creator, I don't see it as a significant flaw. However, it does of course harm curators. I tend to disagree on the point about frontends @0xleastwood though, it seems to me that if this were to be a problem, it would be fairly straightforward to point fingers at those abusing the system.

While I understand an attacker would need to call functions directly to set this up, I'm not fully onboard with the idea that we could punish users who abuse the system in such a way. As per the judging guidelines, any issue that leaks value from the protocol is medium risk.

2 — Med (M): vulns have a risk of 2 and are considered “Medium” severity when assets are not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

I think its perfectly find if this issue is acknowledged and you guys have decided to handle it through front-end design. However, it does not take away from the fact that it is still somewhat of an exploit.

[Zer0dot]

Fair enough! We can roll with medium.