TokenStreamConsumer Manual Review Findings

TSC-01M: Insecure Caching System

Type	Severity	Location
Logical Fault		TokenStreamConsumer.sol:L14-L19, L21-L23

Description:

The TokenStreamConsumer employs a transient-storage based cache system to make input amount computations optimal by requiring them to be calculated once during the execution of a particular transaction.

Based on the structure of the overall project, this approach is insecure as the pending input rewards of a particular TokenStreamConsumer might change within the transaction itself, for example due to being the result of AMM fees as is the case of the UniswapV2Bribe or UniswapV3Elector contracts.

Impact:

The caching system employed for TokenStreamConsumer implementations is incorrect, as the TokenStreamConsumer contract itself has no knowledge of the input calculations and the actual calculations implemented by the project can change within the same transaction.

Example:

Copy
36function _pendingInputAmount(address token) internal view returns (uint128) {
37    (bool cached, uint128 cache) = readCache(token);
38    if (cached) return cache;
39
40    return _pendingInputAmountUncached(token);
41}
42
43function _pendingInputAmountWithUpdate(address token) internal returns (uint128) {
44    (bool cached, uint128 cache) = readCache(token);
45    if (cached) return cache;
46
47    uint128 fetched = _pendingInputAmountWithUpdateUncached(token);
48    writeCache(token, fetched);
49    return fetched;
50}

Recommendation:

We advise the caching system to be removed as the gas savings that it results in are negligible in comparison to the security concerns that arise from it.

Alleviation (17c8d4e59f398021156f6f9657ff278aae0462ae):

The Kyo Finance team evaluated the exhibit and clarified that they do not envision any issues to arise from the caching system as presently implemented in the codebase.

After evaluating their responses, we outlined several instances whereby the cache system of the codebase relies on external assumptions and that there are certain programming approaches that do not match the intended use of the system.

After we proposed several ways via which the codebase can be refactored to avoid these preconditions, the Kyo Finance team opted to retain the current implementation in place as they wish to acknowledge the risk of a derivative contract improperly integrating with the cache system.

As such, we consider the exhibit safely acknowledged as the current implementation of the cache system is secure.