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

Wrong operation #109

Closed
howlbot-integration bot opened this issue Sep 16, 2024 · 2 comments
Closed

Wrong operation #109

howlbot-integration bot opened this issue Sep 16, 2024 · 2 comments
Labels
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 downgraded by judge Judge downgraded the risk level of this issue duplicate-148 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) 🤖_09_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L93

Vulnerability details

Impact

In function _onTransferReceived
...
require(
data != IERC721TokenReceiver.onERC721Received.selector,
"bad nft transfer received data"
);
}

It's wrong logic. Need to check if data is selector. Use == instead of !=

Recommended Mitigation Steps

require(
data == IERC721TokenReceiver.onERC721Received.selector,
"bad nft transfer received data"
);

Assessed type

Invalid Validation

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_09_group AI based duplicate group recommendation bug Something isn't working duplicate-55 sufficient quality report This report is of sufficient quality labels Sep 16, 2024
howlbot-integration bot added a commit that referenced this issue Sep 16, 2024
@c4-judge c4-judge added duplicate-148 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed duplicate-55 3 (High Risk) Assets can be stolen/lost/compromised directly labels Sep 23, 2024
@c4-judge
Copy link
Contributor

alex-ppg changed the severity to 2 (Med Risk)

@c4-judge c4-judge added the partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) label Sep 23, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as partial-50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 downgraded by judge Judge downgraded the risk level of this issue duplicate-148 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) 🤖_09_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant