Omniscia Trustworks Audit

Multisig Static Analysis Findings

Multisig Static Analysis Findings

MUL-01S: Ignored Return Value

TypeSeverityLocation
Standard ConformityMinorMultisig.sol:L158

Description:

The linked statement performs a BEP20 transfer without evaluating the returned bool.

Example:

Contracts/Multisig.sol
146function transferToken(address _recipientForToken, IBEP20 _tokenToTransfer, uint256 _amountToTransferToken) external nonReentrant {
147 require(msg.sender == owner1, "Only owner1 can transferToken");
148 require(canTransferToken, "Trnsfer is not initialized");
149 require(recipientForToken != address(0));
150 require(tokenToTransfer != IBEP20(0));
151 require(_recipientForToken == recipientForToken, "Recptient is not the same as the in the initialized recipient");
152 require(_tokenToTransfer == tokenToTransfer, "tokenToTransfer is not the same as the in the initialized tokenToTransfer");
153 require(_amountToTransferToken == amountToTransferToken, "amountToTransfer is not the same as the in the initialized amountToTransfer");
154 canTransferToken = false;
155 tokenToTransfer = IBEP20(0);
156 amountToTransferToken = 0;
157 recipientForToken = address(0);
158 IBEP20(_tokenToTransfer).transfer(_recipientForToken, _amountToTransferToken);
159}

Recommendation:

We advise that the _tokenToTransfer variable is wrapped in a SafeBEP20 implementation via which the safeTransfer function is invoked proper evaluating whether the call succeeded.

Alleviation:

The ERC-20 transfer call was wrapped with a safeTransfer invocation of the SafeBEP20 library.

MUL-02S: Mislocated Checks

TypeSeverityLocation
Input SanitizationInformationalMultisig.sol:L149, L150, L172

Description:

The linked checks evaluate whether any of the addresses involved in a particular transaction are zero and halt execution at such a state.

Example:

Contracts/Multisig.sol
162function initializeTransferBNB(address payable _recipientForBNB, uint256 _amountToTransferBNB) external nonReentrant {
163 require(msg.sender == owner2, "Only owner2 can initializeTransfer");
164 recipientForBNB = _recipientForBNB;
165 amountToTransferForBNB = _amountToTransferBNB;
166 canTransferBNB = true;
167}
168
169function transferBNB(address payable _recipientForBNB, uint256 _amountToTransferBNB) external nonReentrant {
170 require(msg.sender == owner1, "Only owner1 can transferToken");
171 require(canTransferBNB, "Trnsfer is not initialized");
172 require(recipientForBNB != address(0));
173 require(recipientForBNB == _recipientForBNB, "Recptient is not the same as the in the initialized recipient");
174 require(amountToTransferForBNB == _amountToTransferBNB, "amountToTransfer is not the same as the in the initialized amountToTransfer");
175 canTransferBNB = false;
176 amountToTransferForBNB = 0;
177 recipientForBNB = address(0);
178 _recipientForBNB.transfer(_amountToTransferBNB);
179}

Recommendation:

We advise these checks to instead be imposed on the initialize prefixed functions as these are meant to sanitize the inputs of a valid transaction and the executor should only actuate one, not validate its correctness.

Alleviation:

The zero-address checks were properly relocated to the initialize prefixed functions.

MUL-03S: Variable Mutability Specifier

TypeSeverityLocation
Indeterminate CodeInformationalMultisig.sol:L128

Description:

The owner2 variable is assigned to only once at its declaration.

Example:

Contracts/Multisig.sol
127address payable public owner1;
128address payable public owner2 = 0xE298a311949745b7009174A9bD7c990ffE3Eea5E;
129
130constructor() {
131 owner1 = msg.sender;
132}

Recommendation:

We advise that the constant mutability specifier is introduced to significantly reduce the gas cost involved in utilizing the variable.

Alleviation:

The owner2 was properly set as a constant variable declaration.