0xHelium - Malicious user can spam create profiles for other users

[sherlock-admin2]

0xHelium

medium

Malicious user can spam create profiles for other users

The Registry.createProfile() allow any person to create profile for anyone else without any limitation.This open door for spammer to show off their spam artist talents.

Vulnerability Detail

The Registry.createProfile() allow any person to create profile for anyone else without any limitation. Because of the way this protocol intend to be used , the person who the malicious user created the profile for can use that profile just by having the Id whiich was created from the keccak256 hash of the malicious user account and a random nonce; Although the the person who the malicious user created the profile for is not directly exposed to a loss of funds, this is still an issue as a spammer can create up to type(uint256).max number of profiles for the same victim account.

Impact

Copy and paste this function into Registry.t.sol

function test_MaliciousUsersCreateProfileForOthers(
        string memory _name,
        address malicious_user,
        address owner1
    ) public {
        vm.assume(malicious_user != address(0));
        vm.assume(owner1 != address(0));
        vm.assume(owner2 != address(0));

        vm.startPrank(malicious_user);
        uint256 _nonce = 123;
        uint256 _nonce2 = 1234;
        bytes32 profileId_1 = registry().createProfile(_nonce, _name, metadata, owner1, profile1_members());
        bytes32 profileId_2 = registry().createProfile(_nonce2, _name, metadata, owner1, profile1_members());

        Registry.Profile memory profile1 = registry().getProfileById(profileId_1);
        Registry.Profile memory profile2 = registry().getProfileById(profileId_2);

        assertEq(profile1.id, profileId_1);
        assertEq(profile2.id, profileId_2);

    }

and run the test, it passes , so a spammer can create as many profile as he wants for his victims, and if by any mean he can trick the victims into using the profile Id, there is a possibility that he scams the victims.

Code Snippet

https://github.com/sherlock-audit/2023-09-Gitcoin/blob/main/allo-v2/contracts/core/Registry.sol#L119-L168

Tool used

Manual Review

Recommendation

I recommend ensuring only the caller can create a profile for owner param. This can be done by reverting the function if owner param is not msg.sender.

[sherlock-admin]

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

n33k commented:

invalid, expected behavior

[neeksec]

This is a low severity finding.

spammer can create up to type(uint256).max number of profiles for the same victim account

Creating up to type(uint256).max number of profiles costs too much gas that no one can afford.

For owner to use the malicious profile, an extra spam step is required. So I would categorize it as low severity.

[quentin-abei]

Escalate

• This is not an expected behavior as @neeksec understood at first because sponsor confirmed it.
• type(uint256).max is just the theorical number of spam profile one can create, it doesn't mean that one have to create all that in order to setup his scam, he can well only create one or two.So the gas cost argument is irrelevent.
• Since this is not expected behavior and sponsor will fix it means that it exposes a function malfunction i.e function does not work as intended, and when function does not work as intended it's automatically a valid medium.
• Because of all this: this issue is well a valid medium.

[sherlock-admin2]

Escalate

• This is not an expected behavior as @neeksec understood at first because sponsor confirmed it.
• type(uint256).max is just the theorical number of spam profile one can create, it doesn't mean that one have to create all that in order to setup his scam, he can well only create one or two.So the gas cost argument is irrelevent.
• Since this is not expected behavior and sponsor will fix it means that it exposes a function malfunction i.e function does not work as intended, and when function does not work as intended it's automatically a valid medium.
• Because of all this: this issue is well a valid medium.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.

[thelostone-mc]

Would call this out as a medium severity @quentin-abei

[neeksec]

Escalate

on behalf of sponsor and watson. Since sponsor agree with a medium.

[sherlock-admin2]

Escalate

on behalf of sponsor and watson. Since sponsor agree with a medium.

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.

[nevillehuang]

Would call this out as a medium severity @quentin-abei

This statement below says if creator can trick the victims he can scam them, but it has no relation to any issue within the protocol at all. Unless the watson can prove a explicit loss of funds/breaking of core functionality due to this issue, I am finding it hard to see the medium severity for this issue, because the necessary access control is in place for profiles and the creator will just spend a ton of unnecessary gas. Could you care to elaborate @thelostone-mc ?

so a spammer can create as many profile as he wants for his victims, and if by any mean he can trick the victims into using the profile Id, there is a possibility that he scams the victims.

[quentin-abei]

Let me explain how a malicious user can cause a loss of funds with this exploit:
For the needs of this demo i will call malicious user userA.

• UserA create a new profile with vitalik address as owner, and his own addresses as members.
• UserA then proceed to create a new pool and set his addresses as managers in the pool.
• UserA will now use vitalik address as a proof of legitimacy to ask others users to fund his pool.
• Others users funds the pool of UserA thinking that they are funding a project vitalik own.
• UserA use his managers account to withdraw any amount from the pool.
• Done user have used vitalik address as owner to scam people's.

Because of the above steps, i consider my issue at least a valid medium.
@thelostone-mc , @neeksec

[nevillehuang]

Escalate

I still don’t see how its Medium severity. It is users responsibility to verify that they are funding the correct pool. Even with the fix implemented by sponsor, if malicious profile creators intend to scam users, they can still market themselves as ‘vitaliks’ address. IMO this falls under user input error and my discussion here is done, will let @hrishibhat decide.

[sherlock-admin2]

Escalate

I still don’t see how its Medium severity. It is users responsibility to verify that they are funding the correct pool. Even with the fix implemented by sponsor, if malicious profile creators intend to scam users, they can still market themselves as ‘vitaliks’ address. IMO this falls under user input error and my discussion here is done, will let @hrishibhat decide.

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.

[quentin-abei]

This will be also my final say:
@nevillehuang is confusing himself by saying it falls under user input error:

• User does not input anything here, instead user funds a pool if he loves the owner or the project behind the pool.
• If user funds a pool with pool owner as vitalik and later finds out that even vitalik didn't know about this pool, he will accuse only one person => the Allo team, because it's their responsibility to prevent such things.
• What define a correct pool is a pool, it's simple.
• If a pool have a recognized address as owner, people's will just fund it, not every users are techies.
• Access control on withdraw methods allow managers to withdraw => but pool managers can be added by a profile member when creating a profile => profile members contains malicious user addresses.
• If there where some access control to restrict pool creation to only profile owner, this would have been a non issue.
• But there are none, so this issue exists and lead to potentially lost of funds for users.

I'am writing this with a little heartbreak because i asked @nevillehuang to escalate this for me after sponsor ack a medium severity, he ghosted me , so i asked @neeksec who accepted to escalate even though he is not okay with the severity, and now @nevillehuang come to try to invalidate this by using irrelevant arguments. This is pure evil!
With this being said , this will be my last comment. i leave @hrishibhat decide the severity of this issue.

[nevillehuang]

Ser with all due respect I did not ghost you, this is my response to you, in fact i do not see any message sent by u to me. Please avoid directing negative emotions/insults because I have never done that to you. Lets keep this positive and only leave factual based opinions here. If you have any personal things to say to me please DM, its all love.

[neeksec]

Suggest to keep the original judging.

This issue indeed assists in scams. Since there is no direct risk to funds and it requires scam steps to steal funds from others, I think this should be considered a low risk.

[quentin-abei]

To add to my already written comments , here is sherlock medium issue criteria :

Medium: There is a viable scenario (even if unlikely) that could cause the protocol to enter a state where a material amount of funds can be lost. The attack path is possible with assumptions that either mimic on-chain conditions or reflect conditions that have a reasonable chance of becoming true in the future. The more expensive the attack is for an attacker, the less likely it will be included as a Medium (holding all other factors constant). The vulnerability must be something that is not considered an acceptable risk by a reasonable protocol team.

• sponsor already disagree with low severity=> it's not an acceptable risk for sponsor .
• This attack have high chances of occuring in the future

Let me explain how a malicious user can cause a loss of funds with this exploit: For the needs of this demo i will call malicious user userA.

• UserA create a new profile with vitalik address as owner, and his own addresses as members.
• UserA then proceed to create a new pool and set his addresses as managers in the pool.
• UserA will now use vitalik address as a proof of legitimacy to ask others users to fund his pool.
• Others users funds the pool of UserA thinking that they are funding a project vitalik own.
• UserA use his managers account to withdraw any amount from the pool.
• Done user have used vitalik address as owner to scam people's.

Because of the above steps, i consider my issue at least a valid medium. @thelostone-mc , @neeksec

I already exolained a very likely case where an attack will occur .

Also from the Sherlock docs :

How to identify a medium issue:
Causes a loss of funds but requires certain external conditions or specific states.
Breaks core contract functionality, rendering the contract useless (should not be easily replaced without loss of funds) or leading to unknown potential exploits/loss of funds. 
Ex: Unable to remove malicious user/collateral from the contract.
A material loss of funds, no/minimal profit for the attacker at a considerable cost

My issue tick all the boxes, it should be a medium.

[MLON33]

allo-protocol/allo-v2#354

[Evert0x]

Planning to keep issue closed.

The issue described is not a solidity issue, it's a user issue.

Similar situation, I can create a Uniswap pool and remove all LP tokens. This is not a bug in Uniswap.

[quentin-abei]

Planning to keep issue closed.

The issue described is not a solidity issue, it's a user issue.

Similar situation, I can create a Uniswap pool and remove all LP tokens. This is not a bug in Uniswap.

The issue you are trying to compare this issue with does simply not match .
On uniswap you can not create a pool for another address and add some members to your pool that have special permissions .
If you were able to do this it would be an issue .
Here you are able to do it :
You can create a profile for another person and add some members that have special permissions to open a pool for the another person, grant themselves manager role to withdraw any funds sent to the pool.
Maybe i speak english very poorly that only sponsor Can understand ( confirm issue, fix it , and ack a medium severity) and not Sherlock judges.
Sponsor fix allows for example you to create a profile for me but you can't add members to that profile , only me can do it . Why ? because if you could add members to the profile you created for me it would allows you to add some malicious members that will then open a pool on my behalf, set themselves as managers and withdraw any funds sent to that pool.
Allo is set in such a way that it's for "donations" and people's mainly donate to the pool upon seing the pool owner .
I'am done with comments here , i can't understand how sherlock is trying to dismiss an issue the sponsor does not consider an acceptable risk .

[Evert0x]

The issue is not a smart contract issue, but a business logic issue.

[Evert0x]

Result:
Invalid
Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• nevillehuang: accepted
• neeksec: rejected

[quentin-abei]

Since Sherlock is reconsidering it's "final" judgement see here it means it's not "final" anymore and my issue should be reconsidered for a medium at least based on the above comment on discord from jack himself.

Jack comments suggest my issue have been judged based on a two way 50% by chance rule, and this brick fairness during escalations and definitely makes this judgement biased ( based on reason provided by @Evert0x to invalidate my issue.
@jack-the-pug , i wrote this same comment on discord yesterday and it was deleted by someone at sherlock(if it's not you then it's someone else=.
I'am urging sherlock to reconsider validating my issue since there is no factual reason provided for invalidating it, so it should be valid medium as sponsor requested.