code-423n4 / 2023-02-malt-findings

0 stars 0 forks source link

Upgraded Q -> 2 from #3 [1678055596601] #44

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #3 as 2 risk. The relevant finding follows:

[L-01] The _removeContract() function is not properly implemented Impact The contracts array of MaltRepository contract would be messed up, devs and users would not be able to obtain correct contract string names by the public interface.

File: contracts\Repository.sol 26: string[] public contracts; Proof of Concept As shown of L227 of _removeContract(), the currentContract.index member variable has been cleard to 0, so local variable index of L229 will always be 0 too. The lastContract will be moved to contracts0, overwrite the existing first contract name. Meanwhile the desired removing contract name is still in the contracts array.

File: contracts\Repository.sol 223: function _removeContract(string memory _name) internal { 224: bytes32 hashedName = keccak256(abi.encodePacked(_name)); 225: Contract storage currentContract = globalContracts[hashedName]; 226: currentContract.contractAddress = address(0); 227: currentContract.index = 0; 228: 229: uint256 index = currentContract.index; // @audit has been overwritten 230: string memory lastContract = contracts[contracts.length - 1]; 231: contracts[index] = lastContract; 232: contracts.pop(); 233: emit RemoveContract(hashedName); 234: } Recommended Mitigation Steps File: contracts\Repository.sol 223: function _removeContract(string memory _name) internal { 224: bytes32 hashedName = keccak256(abi.encodePacked(_name)); 225: Contract storage currentContract = globalContracts[hashedName]; 226: currentContract.contractAddress = address(0);

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #25

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory