Omniscia Powercity Audit

StabilityPool Static Analysis Findings

StabilityPool Static Analysis Findings

SPL-01S: Inexistent Event Emission

TypeSeverityLocation
Language SpecificStabilityPool.sol:L312

Description:

The linked function adjusts a sensitive contract variable yet does not emit an event for it.

Example:

StabilityPool.sol
305borrowerOperations = IBorrowerOperations(_borrowerOperationsAddress);
306troveManager = ITroveManager(_troveManagerAddress);
307activePool = IActivePool(_activePoolAddress);
308lusdToken = ILUSDToken(_lusdTokenAddress);
309sortedTroves = ISortedTroves(_sortedTrovesAddress);
310priceFeed = IPriceFeed(_priceFeedAddress);
311communityIssuance = ICommunityIssuance(_communityIssuanceAddress);
312pulseX = IERC20(_pulseX);
313
314emit BorrowerOperationsAddressChanged(_borrowerOperationsAddress);
315emit TroveManagerAddressChanged(_troveManagerAddress);
316emit ActivePoolAddressChanged(_activePoolAddress);
317emit LUSDTokenAddressChanged(_lusdTokenAddress);
318emit SortedTrovesAddressChanged(_sortedTrovesAddress);
319emit PriceFeedAddressChanged(_priceFeedAddress);
320emit CommunityIssuanceAddressChanged(_communityIssuanceAddress);

Recommendation:

We advise an event to be declared and correspondingly emitted to ensure off-chain processes can properly react to this system adjustment.

Alleviation (8bedd3b0df6387957e6b8f5d52507e776c1458b0):

The PulseXAddressChanged event has been introduced to the codebase and is now emitted whenever the pulseX is set, addressing this exhibit's concerns.

SPL-02S: Inexistent Sanitization of Input Address

TypeSeverityLocation
Input SanitizationStabilityPool.sol:L291

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:

StabilityPool.sol
283function setAddresses(
284 address _borrowerOperationsAddress,
285 address _troveManagerAddress,
286 address _activePoolAddress,
287 address _lusdTokenAddress,
288 address _sortedTrovesAddress,
289 address _priceFeedAddress,
290 address _communityIssuanceAddress,
291 address _pulseX
292)
293 external
294 override
295 onlyOwner
296{
297 checkContract(_borrowerOperationsAddress);
298 checkContract(_troveManagerAddress);
299 checkContract(_activePoolAddress);
300 checkContract(_lusdTokenAddress);
301 checkContract(_sortedTrovesAddress);
302 checkContract(_priceFeedAddress);
303 checkContract(_communityIssuanceAddress);

Recommendation:

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

Alleviation (8bedd3b0df6387957e6b8f5d52507e776c1458b0):

The _pulseX address is now properly sanitized in the StabilityPool::setAddresses function by passing it in the CheckContract::checkContract modifier, alleviating this exhibit.

SPL-03S: Improper Invocations of EIP-20 transfer / transferFrom

TypeSeverityLocation
Standard ConformityStabilityPool.sol:L853, L1016

Description:

The linked statements do not properly validate the returned bool values of the EIP-20 standard transfer & transferFrom functions. 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:

StabilityPool.sol
853bool success = pulseX.transfer( msg.sender, _amount );

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 (8bedd3b0df6387957e6b8f5d52507e776c1458b0):

The Powercity team has stated that they intend to utilize the PulseX token solely and that it reverts on unsuccessful transfers, rendering an evaluation of the yielded bool redundant.

As such, we consider this exhibit nullified based on the fact that a known failure-reverting EIP-20 asset will be utilized in the referenced invocations.