Incorrect Usage of Safe Arithmetics Library

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L21 https://github.com/code-423n4/2022-05-rubicon/blob/main/contracts/rubiconPools/BathPair.sol#L556

Vulnerability details

BPR-01M: Incorrect Usage of Safe Arithmetics Library

File	Lines	Type
BathPair.sol	L21, L556	Incorrect Library Usage

Description

The codebase utilizes the SafeMath OpenZeppelin library for the uint16 data type while the library is built for the uint256 data type, rendering it ineffective for uint16 types as they are indirectly cast to the uint256 type and thus no actual safe operations are performed within the uint16 bounds.

Impact

Mathematical outputs will be incorrect in case a uint16-bound output is expected as the operations will yield uint256-bound outputs.

Solution (Recommended Mitigation Steps)

We advise either a purpose-built library for the uint16 data type to be coded and utilized by the code, or the statements to be manually evaluated for correctness in the case of the uint16 type to ensure the mathematical operations are performed as expected.

PoC

Issue is deducible by inspecting the relevant lines referenced in the issue, making note of the using statement and comparing it to the OpenZeppelin codebase referenced above.

Tools

Manual inspection of the codebase.

[bghughes]

I believe this is somewhat of a non-issue. "the statements to be manually evaluated for correctness in the case of the uint16 type to ensure the mathematical operations are performed as expected." - on our case this value will never exceed the uint bounds in practice.

Acknowledging bc uint16 is not really needed in the contract.

[HickupHH3]

Agree it's a non-issue, if we can safely assume stratRewardBPS is reasonably set (<= BPS).