Skip to content

Upgraded Q -> 2 from #19 [1677669238372] #37

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

Closed
c4-judge opened this issue Mar 1, 2023 · 3 comments
Closed

Upgraded Q -> 2 from #19 [1677669238372] #37

c4-judge opened this issue Mar 1, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate-22 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-judge
Copy link
Contributor

c4-judge commented Mar 1, 2023

Judge has assessed an item in Issue #19 as 2 risk. The relevant finding follows:

KUMABondToken.sol
KBT-01L: Potential Approval Blacklist Bypass (Affected Lines: L148)
The KUMABondToken::approve function will apply a blacklist check on the to as well as msg.sender contextual arguments of the call, however, the actual owner of the tokenId is not validated in contrast to setApprovalForAll which disallows an approval to be made by a party that is in the blacklist.

While the approval cannot be actuated on by the transferFrom / safeTransferFrom functions as they do validate the from argument, an approval being made on behalf of a blacklisted owner is an undesirable trait. We advise the code to properly apply the blacklist to the owner of the ERC721 asset, preventing circumvention of the checks if an operator (i.e. isApprovedForAll) was created by a blacklisted owner before they were included in the blacklist.

@c4-judge c4-judge added the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Mar 1, 2023
c4-judge added a commit that referenced this issue Mar 1, 2023
@c4-judge c4-judge closed this as completed Mar 1, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Mar 1, 2023

GalloDaSballo marked the issue as duplicate of #22

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Mar 1, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Mar 1, 2023

GalloDaSballo marked the issue as satisfactory

@GalloDaSballo
Copy link

Maintaining 100% because of the clarity in writing

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 duplicate-22 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants