ProtectedNativeTokenGateway Static Analysis Findings

PNT-01S: Redundant Function Implementation

Type	Severity	Location
Gas Optimization		ProtectedNativeTokenGateway.sol:L163-L168

Description:

The referenced ProtectedNativeTokenGateway::fallback function implementation is redundant as Solidity's behaviour is to not include it if it is not explicitly specified in the version utilized.

Example:

Copy
163/**
164 * @dev Revert fallback calls
165 */
166fallback() external payable {
167  revert('Fallback not allowed');
168}

Recommendation:

We advise it to be safely omitted, optimizing the contract's deployment gas cost as well as the legibility of the codebase.

Alleviation:

The redundant ProtectedNativeTokenGateway::fallback function has been omitted as advised.

PNT-02S: Inexistent Sanitization of Input Address

Type	Severity	Location
Input Sanitization		ProtectedNativeTokenGateway.sol:L30-L40

Description:

The linked function accepts an address argument yet does not properly sanitize it.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
30constructor(
31  address wbnb,
32  address pwbnb,
33  IPool pool
34) {
35  WBNB = IWBNB(wbnb);
36  pWBNB = IPERC20(pwbnb);
37  POOL = pool;
38  WBNB.approve(address(pwbnb), type(uint256).max);
39  IERC20(pwbnb).approve(address(pool), type(uint256).max);
40}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that the address specified is non-zero.

Alleviation:

The input pool address of the ProtectedNativeTokenGateway::constructor is now adequately sanitized via a newly introduced UtilLib and specifically its UtilLib::checkNonZeroAddress function.

While the exhibit has been addressed, we would like to denote that validation of the wbnb and pwbnb addresses is redundant due to the external calls that are performed to them within the ProtectedNativeTokenGateway::constructor itself.

PNT-03S: Improper Invocations of EIP-20 transferFrom

Type	Severity	Location
Standard Conformity		ProtectedNativeTokenGateway.sol:L75, L105

Description:

The linked statements do not properly validate the returned bool of the EIP-20 standard transferFrom function. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
75apWBNB.transferFrom(msg.sender, address(this), amountToWithdraw);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

The Kinza Finance team has stated that they wish to not sanitize the yielded bool values as the tokens they intend to integrate with will either revert or yield true.

We consider the gas overhead of evaluating the yielded bool to be minimal and as such will mark this exhibit as acknowledged.