DittoETH

Summary

The transferFrom function in ERC721Facet.sol always reverts because of caller requirement check.

Vulnerability Details

The transferFrom function in ERC721Facet.sol always reverts because of the check

the function requires msg.sender = from to pass the check. In that case !s.isApprovedForAll[from][msg.sender] will be nothing but !s.isApprovedForAll[msg.sender][msg.sender]

But when we look into the function isApprovedForAll the function sets the owner to operator but not owner to owner or operator to operator. So function transferFrom will always fail.

Impact

transferFrom will fail when combining shorts into one because that’s where it’s been used in the codebase. Also it’s even more critical because when combining a short, a check requires the 0 index or first short to combine with to have a valid NFT record that matches the shorter. If an NFT can’t be transferred for any reason, it breaks the check.

Tools Used

VS Code, Manual Review

Recommendations

Remove that check so that operator can call the function.