AL Mabrook Financials Inc

QuillAudits15 Jan 2024Files named in the report

Audit · firm’s term: audit report

QuillAudits reviewed Mabrook PRE ICO (AL Mabrook Financials Inc) on 15 Jan 2024: 8 findings, 0 critical; 6 marked fixed, 2 open.

No open critical findings

Open the report Open Mabrook PRE ICO’s security record

Report hosted on quillaudits.com.

Trustblock does not review code.

Findings8

Statuses are the security firm’s own, from its report.

Severity	Open	Fixed
Critical	0	0
High	0	2
Medium	0	1
Low	2	3

HighMissing critical input validation check and failure to invoke Ownable_init in the init function causes the protocol to lose ownershipFixed

The Token contract inherits IToken, AgentRoleUpgradeable, and the TokenStorage. When the init function is invoked, all of the state variables would be updated and also the state variables in the inherited contract. In this case, where the Ownable_init() function from the OwnableUpgradeable is not invoked in the init function from the Token contract, the _owner address would be address zero since it was never invoked and it is impossible to invoke afterwards since the OwnableUpgradeable is an abstract contract. This function could have identified the issue if it has employed critical input sanitization to ensure that these addresses are not null addresses.

HighMissing Modifier Restriction on Launch and deployEscrow functionsFixed

These are two critical functions to the protocol that allows for the protocol owners to create new instances of the Escrow and Almabrook contracts. However, these functions do not have a modifier restriction to allow only privileged addresses to invoke. This implies that malicious users can create multiple Escrow and Almabrook contracts for malicious purposes.

MediumSome new projects may end one second after launch timeFixed

The creation of Almabrook contract clone is done with the launch function. This function checks some key properties that make up the Almabrook contract. While the first check appropriately ensures that _startAt can either be equal to the block.timestamp or greater, this implies that the Almabrook activities should begin immediately after launch or after a period of time set in the future. However, the second check ensures that _endAt time can be equal to the _startAt or greater than. This is where the vulnerability arises. If at any point in time, _endAt == _startAt, the project ends after one second.

LowDouble onlyOwner check will increase the cost of gasFixed

The cancel function has an onlyOwner modifier that checks that creator == msg.sender. After this check, this check was again checked for the second time. The gas cost for calling this function will charge double for this check and invariably increase gas fee.

LowPublic function should be marked as external to save gasFixed

The getBalance function is a function in Almabrook declared as public to get the balance of an address. However, this function was not invoked within the contract. In order to save gas invoking this function, it is recommended to mark this function as external.

LowRename state variable escrowId to be different from individual id in Escrow structFixed

The escrowId is a state variable that tracks all of the created escrows. However, this state variable gets updated every time an escrow is deployed and gets set as the id for an individual escrow.