Smart Contract Review of the POH UBI Token

In this article, we performed a smart contract review of the UBI Proof of Humanity Token. We reviewed the source code for the contracts available in this repository, on the master branch, at commit f3e8cec90b82fc7a795cc573a83eb2c7da6d664c.

Proof of Humanity is a system that employs webs of trust, reverse Turing tests, and dispute resolution to create a list of verified humans resistant to sybil attacks. The system aims to ensure that only individuals who are truly human can be added to the list by utilizing webs of trust, where individuals must be verified by multiple trusted individuals within the network. Reverse Turing tests are also employed to confirm that the individuals being verified are not bots or AI. If an individual is found to be a bot or AI, they will be removed from the list. Dispute resolution is also implemented to address any disputes that may arise during the verification process. The ultimate goal of the system is to create a list of verified humans that can be used for various purposes such as voting, participating in decentralized platforms, and more. It was built by Kleros, a Web3 disputes arbitration system.

Project consists of 850 lines of solidity code, most corresponding to the UBI token, with some code assigned to newer versions of streaming and flow versions of the UBI token. Those versions appear to be unfinished. Due to the size of the project, generally the time required to audit and report is estimated to be two weeks.

Main contracts are:

• contracts/UBI.sol: Main UBI token
• contracts/UBIProxy.sol: Proxy interface
• contracts/fUBI.sol: Flow-style streamable UBI token
• contracts/sUBI.sol: Stream-style UBI token

Findings:

Old Solidity version.

Location:

All contracts

All the contracts use a very old Solidity language version (0.7.3). This version has many known vulnerabilities. It is recommended to upgrade to the latest version (0.8.17), as the UBI token is upgradeable.

Disable initializers

As per the OpenZeppelin’s upgradeable contracts documentation, you should not leave an implementation contract uninitialized, as it can be taken over by an attacker.

Recommendation:

To prevent the implementation contract from being used, you should invoke the _disableInitializers function in the constructor to automatically lock it when it is deployed:

constructor() {
_disableInitializers();
}

Arbitrary contract call

Location:

fUBI.createDelegation() (contracts/fUBI.sol:71)

The createDelegation() function calls _safeMint():

_safeMint(recipient, lastTokenId);

This function will call a function on the recipient address, and because the function do not follow the Check-Effect-Interaction (There is an state-change after the _safeMint() call) an attacker could call any public function on the UBI token and access the ubiOutflow[] and ubiInflow[] arrays before they are updated. This is mitigated by the function implementing a reentrancy guard. However, the attacker can still call the function _beforeTokenTransfer() (by transferring a token). This has very little actual impact, as the attacker can only cause a revert in the _beforeTokenTransfer() function, but it is dangerous to leave this the possibility to an eventual attacker.

Recommendation:

This is a minimal impact vulnerability as can be only caused by a privileged address (only the UBI token address), nevertheless, can be avoided by

1. Following the Check-Effect-Interaction pattern and
2. Adding a reentrancy guard to the _beforeTokenTransfer() function

Lack of Check-Effect-Interaction pattern

Location:

•    UBI._withdrawFromDelegation() (contracts/UBI.sol:420),
•    UBI.createDelegation() (contracts/UBI.sol:399) and
•    UBI.cancelDelegation() (contracts/UBI.sol:347):

The function _withdrawFromDelegation() calls two methods from the ‘delegator‘ contract address passed as parameter. A similar pattern is present at the function cancelDelegation() with the ‘delegatorImpl’ parameter and in the function createDelegation() with the parameter ‘implementation’.

This would allow this arbitrary contract to interact with the rest of the token functions before the function has finished updating the internal state values. This can be mitigated in two ways:

1. Do not allow the delegator address to be arbitrary (This is correctly implemented in the code)
2. Implement the Check-Effect-Interaction pattern (This is not implemented)

Recommendation:

Follow the Check-Effect-Interaction pattern in the implementation of all the functions that call external code.