Binary Vault for Price Game #30
Conversation
|
|
||
| // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . | ||
| if (prevOwnershipPacked & _BITMASK_NEXT_INITIALIZED == 0) { | ||
| uint256 nextTokenId = tokenId + 1; |
There was a problem hiding this comment.
Warning: This declaration shadows an existing declaration function nextTokenId(). Change variable name.
|
|
||
| // If the next slot may not have been initialized (i.e. `nextInitialized == false`) . | ||
| if (prevOwnershipPacked & _BITMASK_NEXT_INITIALIZED == 0) { | ||
| uint256 nextTokenId = tokenId + 1; |
There was a problem hiding this comment.
Warning: This declaration shadows an existing declaration function nextTokenId(). Change variable name.
| * @param user Receipt for share | ||
| * @param amount Underlying token amount | ||
| */ | ||
| function addNewLiquidityPosition(address user, uint256 amount) |
There was a problem hiding this comment.
Severity: High
Confidence: High
BinaryVault.addNewLiquidityPosition(address,uint256) uses arbitrary from in transferFrom: underlyingToken.safeTransferFrom(user,address(this),amount)
Exploit Scenario:
function a(address from, address to, uint256 amount) public {
erc20.transferFrom(from, to, amount);
}
Alice approves this contract to spend her ERC20 tokens. Bob can call a and specify Alice's address as the from parameter in transferFrom, allowing him to transfer Alice's tokens to himself.
Recommendation
Use msg.sender as from in safeTransferFrom. Remove user parameter in function and use msg.sender instead.
| * @param amount Underlying token amount | ||
| * @param tokenId nft id to be added liquidity. This should be existing id. | ||
| */ | ||
| function addLiquidityPosition(address user, uint256 amount, uint256 tokenId) |
There was a problem hiding this comment.
Severity: High
Confidence: High
BinaryVault.addLiquidityPosition(address,uint256,uint256) uses arbitrary from in safeTransferFrom: underlyingToken.safeTransferFrom(user,address(this),amount)
Exploit Scenario:
function a(address from, address to, uint256 amount) public {
erc20.transferFrom(from, to, amount);
}
Alice approves this contract to spend her ERC20 tokens. Bob can call a and specify Alice's address as the from parameter in transferFrom, allowing him to transfer Alice's tokens to himself.
Recommendation
Use msg.sender as from in safeTransferFrom. Remove user parameter in function and use msg.sender instead.
| * | ||
| * Emits a {Transfer} event for each mint. | ||
| */ | ||
| function _mint(address to, uint256 quantity) internal virtual { |
There was a problem hiding this comment.
Severity: High
Confidence: High
BinaryVault._mint(address,uint256) contains an incorrect shift operation:
_packedAddressData[to] += quantity * ((1 << _BITPOS_NUMBER_MINTED) | 1)
Recommendation: Swap the order of parameters.
| * | ||
| * Emits a {Transfer} event. | ||
| */ | ||
| function _burn(uint256 tokenId, bool approvalCheck) internal virtual { |
There was a problem hiding this comment.
Severity: High
Confidence: High
BinaryVault._burn(uint256,bool) contains an incorrect shift operation:
_packedAddressData[from] += (1 << _BITPOS_NUMBER_BURNED) - 1
Recommendation: Swap the order of parameters.
| * | ||
| * Returns whether the call correctly returned the expected magic value. | ||
| */ | ||
| function _checkContractOnERC721Received( |
There was a problem hiding this comment.
Severity: Medium
Confidence: Medium
BinaryVault._checkContractOnERC721Received(address,address,uint256,bytes) ignores return value by ERC721A__IERC721Receiver(to).onERC721Received(_msgSenderERC721A(),from,tokenId,_data)
Recommendation: Ensure that all the return values of the function calls are used.
No description provided.