Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash for Project is not controlled - projects with the same hash are possible, and NFTs with the same URI #120

Open
code423n4 opened this issue Aug 5, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons valid

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/HomeFi.sol#L210-L232

Vulnerability details

Impact

Inputed _hash for HomeFi.sol is the URI for NFTs. Contracts do not check duplicates and do not check is it correct or not. It is not dangerous within smart-contracts (as NFT are not used much here), but it can be important together with frontend or backend. Hashes will not be unique.

Proof of Concept

createProject() https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/HomeFi.sol#L210-L232
mintNFT() https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/HomeFi.sol#L284-L297

Tools Used

Hardhat

Recommended Mitigation Steps

Check is it necessary for NFT hashes to be unique - if so, write checks.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 5, 2022
code423n4 added a commit that referenced this issue Aug 5, 2022
@parv3213 parv3213 added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Aug 17, 2022
@jack-the-pug jack-the-pug added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax valid and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons valid
Projects
None yet
Development

No branches or pull requests

3 participants