PaymentPortal Manual Review Findings

PPL-01M: Flash Loan Susceptibility

Description:

The amount that a user needs to pay in ALBT can be manipulated via flash loans as the calculateUSDTtoALBT function entirely relies on the spot price of ALBT which is fully susceptible to manipulation.

Example:

Copy
208// Calculate the amount of ALBT you will get for an amount of USDT
209function calculateUSDTtoALBT(uint256 _usdtAmount) internal view returns (uint256) {
210    (uint256 usdtReserve, uint256 albtReserve, ) = IUniswapV2Pair(uniswapPair).getReserves();
211
212    require(_usdtAmount > 0, 'Insufficient USDT amount');
213    require(albtReserve > 0 && usdtReserve > 0, 'Insufficient liquidity');
214
215    return (albtReserve * _usdtAmount) / usdtReserve;
216}

Recommendation:

We advise a more robust pricing mechanism to be utilized, such as via the usage of decentralized oracle or a Time-Weighted Price Average (TWAP) to ensure that the price is not susceptible. We should note that the likelihood of this being exploited is directly correlated to the amount of funds necessary to procure a membership. If the manipulation yields a reduction in the subscription price that outweighs the flash-loan fee, then this would be considered a valid attack vector. We strongly recommend this particular trait to be assessed by the Alliance Block team and remediated if necessary.

Alleviation:

The calculateUSDTtoALBT function now integrates with a Chainlink ALBT/USDT feed and is no longer susceptible to a flash-loan attack. We should note that the Chainlink integration mechanism does not validate data staleness via the answeredInRound and roundID variables, however, the Alliance Block team reached out to Chainlink and validated that such a conditional is unnecessary for the particular data feeds of the project.

PPL-02M: Incorrect Uniswap Output Calculation

Description:

The calculateUSDTtoALBT function is meant to calculate the amount of ALBT you will acquire by trading in an amount of USDT, however, the calculation does not factor in the fee applied to the exchange nor the increased usdtReserve due to the newly traded units.

Example:

Copy
208// Calculate the amount of ALBT you will get for an amount of USDT
209function calculateUSDTtoALBT(uint256 _usdtAmount) internal view returns (uint256) {
210    (uint256 usdtReserve, uint256 albtReserve, ) = IUniswapV2Pair(uniswapPair).getReserves();
211
212    require(_usdtAmount > 0, 'Insufficient USDT amount');
213    require(albtReserve > 0 && usdtReserve > 0, 'Insufficient liquidity');
214
215    return (albtReserve * _usdtAmount) / usdtReserve;
216}

Recommendation:

We advise the calculations to be adjusted to conform to the getAmountOut paradigm of Uniswap V2's library as used by its router implementation to ensure correct amount calculations.

Alleviation:

The output now properly factors in the Uniswap fee of 0.3% in the trade output.

PPL-03M: Inexistent User Slippage Validation

Description:

The payALBT and payUSDT functions are susceptible to race conditions as they do not impose any slippage validation on the price of the subscription.

Example:

Copy
143// When you pay with ALBT the tokens are transfered directly
144function payALBT(address walletToGiveAccess) public {
145    require(!hasAccess(walletToGiveAccess), 'Already has access');
146
147    uint256 albtAmount = calculateUSDTtoALBT(priceWithALBT);
148
149    transferPayment(msg.sender, albtAmount);
150
151    access[walletToGiveAccess] = block.timestamp + subscriptionDuration;
152}
153
154// When you pay with USDT the tokens are transfered to this contract, then swapped to ALBT, and then transfered
155function payUSDT(address walletToGiveAccess) public {
156    require(!hasAccess(walletToGiveAccess), 'Already has access');
157
158    SafeERC20.safeTransferFrom(IERC20(usdtToken), msg.sender, address(this), priceWithUSDT);
159
160    address[] memory route = new address[](2);
161    route[0] = usdtToken;
162    route[1] = albtToken;
163
164    // Expected amount of ALBT to get back, used to calculate slippage
165    uint256 albtAmount = calculateUSDTtoALBT(priceWithUSDT);
166
167    IERC20(usdtToken).approve(uniswapRouter, priceWithUSDT);
168
169    // TODO: make slippage configurable
170
171    uint256[] memory amounts = IUniswapV2Router01(uniswapRouter).swapExactTokensForTokens(
172        priceWithUSDT,
173        (albtAmount * (1000 - maxSlippage)) / 1000, // max % slippage
174        route,
175        address(this),
176        block.timestamp + 3600 * 24
177    );
178
179    uint256 exactALBTAmount = amounts[amounts.length - 1];
180
181    transferPayment(address(this), exactALBTAmount);
182
183    access[walletToGiveAccess] = block.timestamp + subscriptionDuration;
184}

Recommendation:

We advise a new argument to be introduced to both functions that ensures the priceWithALBT and priceWithUSDT is within an expected upper bound that the user is willing to pay as it can be immediately changed by the owner of the contract, a trait that can affect transactions pending acceptance on the blockchain.

Alleviation:

A proper slippage argument was introduced for both payment functions thereby alleviating this exhibit.

PPL-04M: On-Chain Slippage Logical Fallacy

Description:

The usage of maxSlippage as a concept across the contract is a logical fallacy given that the output amount calculated by calculateUSDTtoALBT relies on the actual values the exchange will use, meaning that between the time the albtAmount is measured and the swapExactTokensForTokens exchange is executed the price remains the same and thus the exact output will be achieved by the swap.

Example:

Copy
164// Expected amount of ALBT to get back, used to calculate slippage
165uint256 albtAmount = calculateUSDTtoALBT(priceWithUSDT);
166
167IERC20(usdtToken).approve(uniswapRouter, priceWithUSDT);
168
169// TODO: make slippage configurable
170
171uint256[] memory amounts = IUniswapV2Router01(uniswapRouter).swapExactTokensForTokens(
172    priceWithUSDT,
173    (albtAmount * (1000 - maxSlippage)) / 1000, // max % slippage
174    route,
175    address(this),
176    block.timestamp + 3600 * 24
177);

Recommendation:

We advise the concept of maxSlippage to be entirely omitted from the codebase as it is ineffectual. As an alternative, we advise the pricing mechanism of calculateUSDTtoALBT to be made robust against manipulations and be used in conjunction with maxSlippage as the slippage argument would be valid in such a case.

Alleviation:

The slippage argument has now been made valid in the codebase as the pricing mechanism the contract utilizes is external to Uniswap, rendering this exhibit null.

PPL-05M: Inexistent Minimum Subscription

Description:

The subscriptionDuration of the contract can be arbitrarily adjusted by the owner of the contract and is initialized at 30 days which the documentation of the project states is the de-facto duration.

Example:

Copy
107function setSubscriptionDuration(uint256 _subscriptionDuration) public onlyOwner {
108    subscriptionDuration = _subscriptionDuration;
109}

Recommendation:

We advise a minimum duration to be imposed here or the duration to only be able to increase as in the current implementation an unwanted race-condition can materialize whereby pending transactions that plan to purchase a membership may have been submitted with a different duration and processed with another one, potentially even zero one.

Alleviation:

A minimum subscription duration of 1 weeks was introduced, ensuring users have a lower bound subscription value that they expect to acquire.

PPL-06M: Inexistent Price Range Bounds

Description:

The setPriceWithALBT and setPriceWithUSDT functions do not perform any input sanitization on the values they are passed in.

Example:

Copy
91function setPriceWithALBT(uint256 _priceWithALBT) public onlyOwner {
92    priceWithALBT = _priceWithALBT;
93}
94
95function setPriceWithUSDT(uint256 _priceWithUSDT) public onlyOwner {
96    priceWithUSDT = _priceWithUSDT;
97}

Recommendation:

We strongly recommend some form of sanitization to be applied, such as that the price in ALBT will always be lower or equal to the price in USDT (a valid logical constraint) or that the price of either type is a minimum of X and a maximum of Y in terms of USDT.

Alleviation:

Logical checks ensuring that the price in ALBT is always less than the price in USDT have been introduced in the codebase.

PPL-07M: Inexistent Slippage Bound Evaluation

Description:

The maxSlippage value is meant to represent a percentage using the numeric range [0,1000], however, no such bounds are validated in its setter function that can cause the payUSDT function to fail execution entirely.

Example:

Copy
103function setMaxSlippage(uint256 _maxSlippage) public onlyOwner {
104    maxSlippage = _maxSlippage;
105}

Recommendation:

We advise an upper-bound require check to be introduced ensuring that _maxSlippage is less-than-or-equal-to (<=) the value of 1000.

Alleviation:

An upper bound check was introduced to the codebase as per our recommendation.