`_decimals` MUST be `_underlying.decimals()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L32-L35

Vulnerability details

Impact

The constructor of the ZcToken contract sets the underlying asset and the number of decimal places of the ZcToken. In case that this value is different to the underlying asset, the user may lose or send more tokens than expected.

Proof of Concept

In the ZcToken contract there are two methods: convertToPrincipal and convertToUnderlying, and in neither of them a conversion is performed taking into account that the decimals of the ERC5095 and the underlying may be different, so if users wants to use them to convert the quantity to transfer, they may get the wrong value and an action may occur in an unwanted amount. The redeem and withdraw methods can produce errors.

Affected source code:

• https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L43-L57

Recommended Mitigation Steps

• Use the underlying's decimal instead of provide a new value.

[JTraversa]

1. They are the same in our implementation, meaning this is not an issue
2. The specifics of the erc-5095 implementation are out of scope

[bghughes]

Given that centralization, risk is acknowledged (e.g. making an off-chain error in creating the ZcToken) and the fact that the functions the warden references are view functions, I believe this is QA.

It is informational because I believe an incorrect constructor call to the ZcToken could cause unintended consequences with respect to decimal precision throughout the protocol.

[bghughes]

Grouping this with the warden’s QA report #164

[bghughes]

Duplicate of #164