The WooPPV2 contract doesn't need to be an admin
When the WooPPV2 contract makes a swap, it calls postPrice
on the WooracleV2_2 to post the new price for future swaps:
IWooracleV2_2(wooracle).postPrice(baseToken, uint128(newPrice));
Here is the implementation of postPrice
:
function postPrice(address _base, uint128 _price) external onlyAdmin {
infos[_base].price = _price;
if (msg.sender != wooPP) {
timestamp = block.timestamp;
}
}
Since postPrice
is onlyAdmin
, this means the wooPP
address must be set as an admin on the oracle contract. However, this means that the wooPP
can call any other admin function, including the fallback function.
There is currently no way for an unprivileged caller to cause WooPPV2 to call the oracle in an unexpected manner without it also reverting. But, since the fallback function accepts many function selectors, a future version of either contract could introduce a potential negative interaction where the pool calls an external contract using a user-controlled address, for instance to transfer an unknown ERC20 token or to make a callback call, and an attacker unexpectedly passes in the address of the oracle instead of the token or receiver address.
Since postPrice
already checks if the caller is wooPP
, it wouldn't cost any more gas to repeat the body of the onlyAdmin
check in the function with an extra case for wooPP
. Alternatively, another function can be added that only allows wooPP
to call it, that posts the price without resetting the timestamp. Either of these two strategies would make it so that wooPP
cannot call any of the other onlyAdmin
functions.
This issue has been acknowledged by WOOFI, and the fix has been implemented in commit ea79a0d↗