masa-finance / masa-contracts-identity Goto Github PK
View Code? Open in Web Editor NEWSmart Contracts for Soul Bound Identities
Home Page: https://masa.finance
License: MIT License
Smart Contracts for Soul Bound Identities
Home Page: https://masa.finance
License: MIT License
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
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
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.
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).
masa-contracts-identity/contracts/SoulStore.sol
Lines 82 to 94 in 112f073
Is this bug a Critical Vulnerability: N
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.
Let users set a Soul Name as the default one for an identity
Just small double spelling, which also affect Docs:
"SoulName id id".
"id" spelled twice.
Describe the bug
This bug allows any current masa domain holder to renew a domain for an unlimited amount of years without paying any funds besides the gas needed to call the smart contract, for both base(.base) and eth(.soul) domains, directly from the main website.
Steps to reproduce the behavior:
The bug in question is easily replicable by visiting https://app.masa.finance/soulnames and connecting a metamask wallet(most likely this works with any wallet provider) holding a soulname, in this case base domain or .soul eth domain.
After that it's required to enter any domain name a user is holding on that particular network in the domain search bar, increase the number of years to renew(the number of years a user can select is unlimited) and click "Renew your soulname".
Disregard the disclaimer "insufficient funds" and renew the domain for any number of years, by confirming the transaction in metamask.
Immediately a confirmation pop-up will show up confirming the successful extended expiration date without a user having to pay any ETH for renewing the domain, besides gas.
I've tested this bug only with 5+ letters domain on both base and eth networks, the devs should test if this works with less than 5 letters domains, but most likely it will work the same.
Expected behavior
When renewing a 5 letter domain, a user should pay a 0.0008 ETH fee multiplied by each year a user intends to renew, while here anyone can renew a domain for unlimited years.
As metamask outlines, the user sends an amount of ETH which is 0, this amount should be higher of course
Screenshots can be found in the additional context section.
Videos
The wallet used in the 2 videos are 0x397dF6c66ddED5742b3F9E20B46e0834084F8F41 for the first video, in this wallet i hold my .base domains.
Regarding the second video the wallet used is 0x5670297049F46489989D0feaA93A5C3D2d53F3c8 where i hold the governance.soul name on Eth mainnet.
Links to the 2 videos are here https://www.youtube.com/watch?v=JqmXFDJw7Uo
and https://www.youtube.com/watch?v=SCXTNqSqHsg
In the first video i forgot to remove night mode, but the video is still clear.
I can't send the videos in file format here as github won't allow me, so check the youtube links
Device Info
Additional context
Metamask HEX data shows the function in question is the "renew years period", the path is this one i'm quite sure contracts/SoulName.sol , you will want to take a look at that, i'm attaching a screenshot for reference.
Another minor UI issue i've encountered:
When renewing a domain and entering that domain name in the search bar, the website always shows the expiration date of that domain as one year from now, said that when renewing the domain multiple times, the years renewed stack up so in the end everything is working as intended here, this is not a critical issue.
I've noticed after renewing for 150+ years the ETH price counter goes back to 0 ETH, i'm attaching a photo for this.
After 100 years the fee for every additional year seems to be way higher, is this intentional?
This might just be an issue with counter on the website, which is a bit buggy, i'll let you take a look at this yourselves.
Is this bug a Critical Vulnerability?
YES
This bug is extremely critical and should be looked into as fast possible, it's on the main website page and replicable by any user, i'm still not sure how the renewal contract went through testing, but there you go!
My contacts are @telog14 on X and telog on Discord and my ENS is telog14.eth pointing to the address 0xA2Fce8470574c5e174c8359809bF596450828FC9 in case of bug bounty reward.
I can pay for the additional years i renewed for free no problem.
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.
Add new Purchase Event to the SoulStore smart contract with the following information:
@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.
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.
This scope needs to be formalized before we ship this to testnet and production.
@miquelcabot to add the current status of V2 in bullet points
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.