Lack of parameter to
check in ERC1155Minimal.sol
makes this contract to be not compliant with ERC-1155 standard
#226
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-500
grade-b
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
🤖_25_group
AI based duplicate group recommendation
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/tokens/ERC1155Minimal.sol#L112-L119
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/tokens/ERC1155Minimal.sol#L163-L170
Vulnerability details
Impact
According to provided documentation,
ERC1155Minimal.sol
isA minimalist implementation of the ERC1155 token standard without metadata
.Docs straightforwardly mention that
ERC1155 used by the protocol
areSFPM and factory ERC1155 tokens
However, the current implementation does not comply with EIP-1155. Since the compliance requirement was straightforwardly mentioned as a requirement in the docs - I've evaluated this issue as Medium.
During the previous Code4rena contests, lack of compliance with EIP-1155 was evaluated as Medium too, e.g.:
Moreover, during the previous C4 contests, lack of EIP compliance was usually evaluated as High/Medium
Lack of compliance may cause unexpected behavior. Other protocols that integrate with contract may incorrectly assume that it's EIP-1155 compliant - especially that documentation states that it's ERC-1155. Any deviation from this standard will broke the composability and may lead to fund loss. While protocol's implements a contract and describes it as ERC-1155, it should fully conform to ERC-1155 standard.
Proof of Concept
According to EIP-1155
None of these functions verify if
_to
is the zero address, which implies that contract is not compliant with EIP-1155.File: ERC1155Minimal.sol
In function
safeTransferFrom()
, when parameterto
is the address zero,to.code.length
will return0
, thus we won't enter above conditional branch. The same issue occurs forsafeBatchTransferFrom()
:File: ERC1155Minimal.sol
This leads to conclusion that it's possible to
safeTransferFrom()
orsafeBatchTransferFrom()
toaddress(0)
, even though EIP-1155 straightforwardly states that everyaddress(0)
transfer should revert. This means that protocol violates EIP-1155.Tools Used
Manual code review
Recommended Mitigation Steps
In both
safeBatchTransferFrom()
andsafeTransferFrom()
add additional line which will verify if parameterto
is not address zero:Assessed type
Invalid Validation
The text was updated successfully, but these errors were encountered: