Omniscia Alliance Block Audit
SubscriptionPaymentPortal Manual Review Findings
SubscriptionPaymentPortal Manual Review Findings
CON-01M: Incorrect Enforcement of Extension Factor
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | SubscriptionPaymentPortal.sol:L379-L386 |
Description:
The extension factor is improperly enforced for new subscriptions in which case any non-FP_ONE value for it will cause new subscriptions to be impossible for the service.
Impact:
New subscriptions will improperly fail if any non-FP_ONE extension factor has been specified.
Example:
355/// @dev safely extend a user's subscription. Handles the possible356/// existence of either a current or expired subscription, and a first time357/// subscription. Whitelisted addresses MAY pay for a subscription, e.g.358/// they may be aware of an imminent de-whitelisting and do not want an359/// interruption in service, so are willing to pay for the subscription. If360/// an address is whitelisted their expiry will be extended relative to361/// their onchain payments ONLY i.e. their whitelisting will not be362/// extended or considered.363/// Internal function with no access checks. Calling context MUST check the364/// payment config is valid and take correct payment.365/// @param paymentConfig_ The paymentConfig_ associated with paying for366/// subscriptions.367/// @param walletToGiveAccess_ The wallet that is extending its paid368/// subscription expiry.369function _extendSubscription(370 PaymentConfig calldata paymentConfig_,371 address walletToGiveAccess_372) internal {373 uint256 currentAccess_ = paidUntil[walletToGiveAccess_].max(374 // solhint-disable-next-line not-rely-on-time375 block.timestamp376 );377 uint256 subscriptionDuration_ = paymentConfig_.subscriptionDuration;378 uint256 newAccess_ = subscriptionDuration_ + currentAccess_;379 require(380 newAccess_ <=381 // solhint-disable-next-line not-rely-on-time382 block.timestamp +383 uint256(paymentConfig_.maximumExtensionFactor)384 .fixedPointMul(subscriptionDuration_),385 "PaymentPortal: MAX_ACCESS"386 );387 paidUntil[walletToGiveAccess_] = newAccess_;388 emit ExtendSubscriptionExpiry(389 msg.sender,390 walletToGiveAccess_,391 newAccess_392 );393}Recommendation:
We advise the extension factor to only be applied when paidUntil is non-zero and the non-zero case to introduce graceful handling for a long-expired subscription that needs to be refreshed.
Alleviation:
The code was instead adjusted to deprecate the extension factor and instead utilized a literal value for the maximum extension a particular subscription can have at a given point in time, alleviating this exhibit and ensuring that the extension limit is calculated using a simple addition instead of a fixed point multiplication.
CON-02M: Incorrectly Calculated TKN Amount
| Type | Severity | Location |
|---|---|---|
| Logical Fault | | SubscriptionPaymentPortal.sol:L412 |
Description:
The calculateUSDTtoTKN function call within payTKN will incorrectly assume a Uniswap fee is meant to be deducted thus producing a number lower than it should be.
Impact:
Incorrect incentives and payment calculations can cause the subscription service user flow (TKN vs USDT) to be unintentionally skewed towards one side of the two tokens.
Example:
395/// When subscriber pays with TKN the tokens are transfered directly from396/// the subscriber to Alice/Bob. The amount of TKN is calculated as per397/// `calculateUSDTtoTKN` from the oracle at the prevailing `priceWithTKN`398/// usdt rate.399/// @param paymentConfig_ The paymentConfig the user believes is in effect400/// when they pay. If any of the terms change due to the owner modifying401/// them the transaction will roll back and the user can try again.402/// @param walletToGiveAccess_ The Wallet that will have their subscription403/// extended when `msg.sender` pays the subscription fee.404function payTKN(405 PaymentConfig calldata paymentConfig_,406 address walletToGiveAccess_407) external onlyValidPaymentConfig(paymentConfig_) nonReentrant {408 _extendSubscription(paymentConfig_, walletToGiveAccess_);409 _transferPayment(410 paymentConfig_,411 msg.sender,412 calculateUSDTtoTKN(paymentConfig_.priceWithTKN)413 );414}Recommendation:
We advise an alternative oracle measurement to be utilized here instead allowing the full TKN amount to be extracted by the payer as it currently causes an additional unintended incentive to pay with TKNs for the subscription.
Alleviation:
The UNISWAP_FEE constant has been relocated from the USDTTKNPriceOracle implementation to the SubscriptionPaymentPortal contract itself, permitting it to be used for the calculation of the return amount for the Uniswap trade as well as fixing the aforementioned TKN conversion as it no longer applies the Uniswap fee by default.