Omniscia Powercity Audit

BorrowerOperations Static Analysis Findings

BorrowerOperations Static Analysis Findings

BOS-01S: Inexistent Event Emission

TypeSeverityLocation
Language SpecificBorrowerOperations.sol:L145

Description:

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

Example:

BorrowerOperations.sol
134troveManager = ITroveManager(_troveManagerAddress);
135activePool = IActivePool(_activePoolAddress);
136defaultPool = IDefaultPool(_defaultPoolAddress);
137stabilityPoolAddress = _stabilityPoolAddress;
138gasPoolAddress = _gasPoolAddress;
139collSurplusPool = ICollSurplusPool(_collSurplusPoolAddress);
140priceFeed = IPriceFeed(_priceFeedAddress);
141sortedTroves = ISortedTroves(_sortedTrovesAddress);
142lusdToken = ILUSDToken(_lusdTokenAddress);
143lqtyStakingAddress = _lqtyStakingAddress;
144lqtyStaking = ILQTYStaking(_lqtyStakingAddress);
145pulseXToken = IPulseX(_pulseXaddress);
146
147emit TroveManagerAddressChanged(_troveManagerAddress);
148emit ActivePoolAddressChanged(_activePoolAddress);
149emit DefaultPoolAddressChanged(_defaultPoolAddress);
150emit StabilityPoolAddressChanged(_stabilityPoolAddress);
151emit GasPoolAddressChanged(_gasPoolAddress);
152emit CollSurplusPoolAddressChanged(_collSurplusPoolAddress);
153emit PriceFeedAddressChanged(_priceFeedAddress);
154emit SortedTrovesAddressChanged(_sortedTrovesAddress);
155emit LUSDTokenAddressChanged(_lusdTokenAddress);
156emit LQTYStakingAddressChanged(_lqtyStakingAddress);

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 pulseXToken is set, addressing this exhibit's concerns.

BOS-02S: Inexistent Visibility Specifiers

TypeSeverityLocation
Code StyleBorrowerOperations.sol:L24, L26, L28

Description:

The linked variables have no visibility specifier explicitly set.

Example:

BorrowerOperations.sol
24address stabilityPoolAddress;

Recommendation:

We advise them to be set so to avoid potential compilation discrepancies in the future as the current behaviour is for the compiler to assign one automatically which may deviate between pragma versions.

Alleviation (8bedd3b0df6387957e6b8f5d52507e776c1458b0):

The stabilityPoolAddress member has been set as public, ensuring that it contains an explicit visibility specifier and thus addressing this exhibit.

BOS-03S: Inexistent Sanitization of Input Address

TypeSeverityLocation
Input SanitizationBorrowerOperations.sol:L114

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:

BorrowerOperations.sol
103function setAddresses(
104 address _troveManagerAddress,
105 address _activePoolAddress,
106 address _defaultPoolAddress,
107 address _stabilityPoolAddress,
108 address _gasPoolAddress,
109 address _collSurplusPoolAddress,
110 address _priceFeedAddress,
111 address _sortedTrovesAddress,
112 address _lusdTokenAddress,
113 address _lqtyStakingAddress,
114 address _pulseXaddress
115)
116 external
117 override
118 onlyOwner
119{
120 // This makes impossible to open a trove with zero withdrawn LUSD
121 assert(MIN_NET_DEBT > 0);
122
123 checkContract(_troveManagerAddress);
124 checkContract(_activePoolAddress);
125 checkContract(_defaultPoolAddress);
126 checkContract(_stabilityPoolAddress);
127 checkContract(_gasPoolAddress);
128 checkContract(_collSurplusPoolAddress);
129 checkContract(_priceFeedAddress);
130 checkContract(_sortedTrovesAddress);
131 checkContract(_lusdTokenAddress);
132 checkContract(_lqtyStakingAddress);

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 BorrowerOperations::setAddresses function by passing it in the CheckContract::checkContract modifier, alleviating this exhibit.

BOS-04S: Improper Invocations of EIP-20 transferFrom

TypeSeverityLocation
Standard ConformityBorrowerOperations.sol:L170, L226, L237, L263

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:

BorrowerOperations.sol
170pulseXToken.transferFrom(msg.sender, address(this), _PulseXAmount);

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.