Broken functionality for ERC1155 `safeTransferFrom` with individual asset approvals in AssetRegister

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/YieldBox/blob/master/contracts/ERC1155.sol#L83

Vulnerability details

AssetRegister inherits YieldBox's ERC1155 contract and adds logic to approve individual token by their ids to specific operators via

mapping(address => mapping(address => mapping(uint256 => bool))) public isApprovedForAsset

To check for individual approvals ERC1155 implementation modifies _requireTransferAllowed function to have additional parameter approved.

function _requireTransferAllowed(address _from, bool _approved) internal view virtual {
    require(_from == msg.sender || _approved || isApprovedForAll[_from][msg.sender] == true, "Transfer not allowed");
}

There are also changes to modify YieldBox's original ERC1155 methods safeTransferFrom

function safeTransferFrom(address from, address to, uint256 id, uint256 value, bytes calldata data) external override {
-    _requireTransferAllowed(from);
+    _requireTransferAllowed(from, false);
     // ...
}

There is additional method defined for Yieldbox transfer that implement transfer logic doing proper check using allowed modifier

// YieldBox.sol
function transfer(
    address from,
    address to,
    uint256 assetId,
    uint256 share
) public allowed(from, assetId) {
    _transferSingle(from, to, assetId, share);
}

// NativeTokenFactory.sol
modifier allowed(address _from, uint256 _id) {
    _requireTransferAllowed(_from, isApprovedForAsset[_from][msg.sender][_id]);
    _;
}

It is impossible to transfer individual YieldBox ERC1155 tokens by approved operators using standard ERC1155 safeTransferFrom method.

Impact

Medium - broken backward compatibility with ERC1155

Proof of concept

User:

• approves spending of token to specific operator

Operator:

• gets revert trying to transfer token using standard safeTransferFrom of ERC1155

Tools Used

Manual review

Recommended Mitigation Steps

• Override safeTransferFrom and safeBatchTransferFrom ERC1155 method to include invidiual token id allowance check once

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora (sponsor) confirmed

[c4-judge]

dmvt marked the issue as selected for report

[c4-judge]

dmvt marked the issue as not selected for report

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[c4-judge]

dmvt marked the issue as grade-b