masa-finance / masa-contracts-identity Goto Github PK

Right now the API can mint more than one identity per address, this should not be the case.

Describe the bug: I believe this is an old non-useful contract but you use Masa token in tests.
mint() function has no onlyOwner modifier and everyone can mint as much $MASA as want.

Is critical: N

┆Issue is synchronized with this Jira Bug by Unito

• Move utility scripts from src/ to other repository

When burning a SoulName NFT, its URI is not properly deleted
This causes that when you burn a SoulName NFT, you can't mint a new one reusing its URI.

Let users set a Soul Name as the default one for an identity

Vulnerable URL: https://github.com/masa-finance/masa-contracts-identity/blob/main/contracts/SoulLinker.sol

Description:
Well, the contract SoulLinker has a solidity version of 0.8.7 and uses a lot of require statements for errors. But a solidity version that is greater than 0.8.4 can use custom errors to save gas. In this contract
the require statement use more gas per iteration. Instead of using strings for error messages, you can use custom
errors to reduce both deployment and runtime gas costs. In addition, they are very convenient as you can easily pass
dynamic information to them. By this, you can save a lot of gas.

┆Issue is synchronized with this Jira Bug by Unito

URL: https://github.com/masa-finance/masa-contracts-identity/blob/main/contracts/SoulboundIdentity.sol

Description:
One possible vulnerability in this code is that the mint function does not check that the caller has enough funds to pay for the cost of minting a new soulbound identity. This means that if the contract has a price associated with minting a new identity, a user could potentially call the mint function and create a new identity without having enough funds to pay for it, effectively creating a new identity for free. This could potentially lead to abuse or unauthorized usage of the contract.

Another possible vulnerability in this code is the lack of a function to check the owner of a soul name. In the mintIdentityWithName function, the contract allows the caller to specify the owner of the new identity, but there is no way to verify that the caller is actually the owner of the specified name. This means that a malicious user could potentially call this function and claim ownership of another user's soul name, potentially leading to unauthorized access or misuse of the contract.

Additionally, the setSoulName function allows the owner of the contract to set the address of the linked soul name contract, but there is no check to ensure that the provided address is actually a valid soul name contract. This means that a malicious user could potentially set the linked soul name contract to an arbitrary address, potentially leading to unexpected behavior or security vulnerabilities.

┆Issue is synchronized with this Jira Bug by Unito

Add new Purchase Event to the SoulStore smart contract with the following information:

• Duration
• Amount of native token
• Currency used
• Total USD

Just small double spelling, which also affect Docs:
"SoulName id id".
"id" spelled twice.

┆Issue is synchronized with this Jira Bug by Unito

Vulnerable URL: https://github.com/masa-finance/masa-contracts-identity/blob/main/contracts/SoulLinker.sol

Description:
One potential vulnerability in this code is that the setSoulboundIdentity function allows the owner to change the address of the soulboundIdentity contract. However, the new contract address is not verified to ensure that it is actually a valid instance of the ISoulboundIdentity contract. This means that an attacker who is able to control the owner account could potentially set the soulboundIdentity contract address to a contract that they own, allowing them to potentially gain control over the functions of the SoulLinker contract.

Is critical: N

┆Issue is synchronized with this Jira Bug by Unito

@miquelcabot @H34D we can we remove non-native paymetns from the scope of the Soulname v2 contract - we have only $400 in USDC payments through the soulstore - we should kill multi-currency support in v2 to simplify code. This is thinking generally about this no matter what network it is on its an unused feature and we can make things simpler by killing the feature.

Subnet context

There is no payment provider on the network (because its completely blank) which means we can not use the Payment gateway we have to do payments for soul names and SBT mints like we do on the other networks. Even when there is a swap provider we would need to decide how we handle liquidity and trading pairs there. Because its most likely that the liquidity is close to zero on a tool chain.

Research scoping

This scope needs to be formalized before we ship this to testnet and production.

• Remove multi-currency support
• Remove dependency on middleware
• Review and update emitted events to consolidate and make simpler for indexing
• Review OpenSea specifications for expired soulnames
• Think about migrating to ENS stack to be compatible with the entire ETH ecosystem

Current V2 Status

@miquelcabot to add the current status of V2 in bullet points

• Renewal with payments

https://www.notion.so/masafinance/Soulname-renewal-Soulstore-v2-contract-3c28979be6cd4ef69e6f7871f511dfc0?pvs=4

• add protocol fee wallet
• add protocol fee amount
• add protocol fee percent

Describe the bug:
Actually it's a problem, not a bug. Since you have no check of entered soulname type, there is a problem:
name "degen.soul" will cost the same, as "❤️.soul" because ❤️ encoding as 0xe29da4efb88f with length of 6 bytes. There are some emoji with length of 4 bytes, for example "😁" stores in 4 bytes which is 0xf09f9881. Some of them stores in over 20 bytes.

Expected behavior: I expected one-emoji soulname price will be around 1-letter soulname price like it costs in ENS (there are minimum length of 3 by the way).

Is this bug a Critical Vulnerability: N

┆Issue is synchronized with this Jira Bug by Unito

• Set metadata endpoints, etc... for Mainnet deployment
• Add authority account to Self-Sovereign SBTs
• Deploy only specific Smart Contracts in Mainnet