code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-02-malt/blob/main/contracts/Repository.sol#L236-L242

Vulnerability details

Impact

Repository._updateContract() is used to change the contract name.

But if this function is called using the _name that doesn't have a trader contract yet, the _name will have a valid contract without adding to contracts array.

Proof of Concept

Repository._updateContract() changes the contract for _name.

File: 2023-02-malt\contracts\Repository.sol
236:   function _updateContract(string memory _name, address _newContract) internal { //@audit should check if _name has a contract address already
237:     require(_newContract != address(0), "0x0");
238:     bytes32 hashedName = keccak256(abi.encodePacked(_name));
239:     Contract storage currentContract = globalContracts[hashedName];
240:     currentContract.contractAddress = _newContract;
241:     emit UpdateContract(hashedName, _newContract);
242:   }

But it doesn't check if _name has a non-zero contract already and it will work unexpectedly if the function is called with a new _name param.

In this case, the _name will have a valid contract but the contract won't be added to contracts array.

Tools Used

Manual Review

Recommended Mitigation Steps

We should revert if _name doesn't have a valid contract.

  function _updateContract(string memory _name, address _newContract) internal {
    require(_newContract != address(0), "0x0");
    bytes32 hashedName = keccak256(abi.encodePacked(_name));
    Contract storage currentContract = globalContracts[hashedName];

    require(currentContract.contractAddress != address(0), "Invalid name"); //++++++++++++++++++

    currentContract.contractAddress = _newContract;
    emit UpdateContract(hashedName, _newContract);
  }