prb-math library is not audited and needs to be carefully trusted

[code423n4]

Lines of code

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L8-L11
https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/UD34x4.sol#L5
https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L5-L6
https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/DrawAccumulatorLib.sol#L6
https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/abstract/TieredLiquidityDistributor.sol#L5-L8
https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L4-L6

Vulnerability details

Impact

In most of the PoolTogether contracts prb-math library is heavily used but the library prb-math documents that it is not audited by a security researcher. This means its more risky to rely on this library.

The prb-math library mentions,
Security
While I set a high bar for code quality and test coverage, you should not assume that this project is completely safe to use. PRBMath has not yet been audited by a third-party security researcher.

Reference link- https://github.com/PaulRBerg/prb-math#security

Therefore, the prb-math library should be audited before its heavy usage in PoolTogether contracts.

Proof of Concept

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/PrizePool.sol#L8-L11

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/UD34x4.sol#L5

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/TierCalculationLib.sol#L5-L6

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/libraries/DrawAccumulatorLib.sol#L6

https://github.com/GenerationSoftware/pt-v5-prize-pool/blob/4bc8a12b857856828c018510b5500d722b79ca3a/src/abstract/TieredLiquidityDistributor.sol#L5-L8

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L4-L6

Tools Used

Manual review

Recommended Mitigation Steps

Consider (crowdsourcing) an audit for prb-math library.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #423

[c4-judge]

Picodes marked the issue as partial-50

[c4-judge]

Picodes marked the issue as unsatisfactory:
Overinflated severity

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

Downgrading to QA as this report doesn't showcase any bug

[0xRizwan]

@Picodes

With due respect to judge decision, I would like to add additional context as below,

This is valid issue and as confirmed in #423 the root cause of the issue is the prb-math library which is not audited. Usually, external libraries which are used in contracts are audited or libraries which are used at project risk should already informed in known issue, however this is not the case here.

Further after digging in prb-math library, i found below issues which are fixed in latest version.

prb-math library library author bug acceptance:
sablier-labs/v2-core#432
PaulRBerg/prb-math#181
PaulRBerg/prb-math#179

It is good to know that, prb-math will add a new directory "audits" performed by Cantina(a marketplace by spearbit).

It is further recommended to update the prb-math library to latest version v4.0.1

I think the issue deserves to be particially satisfactory to Medium.

Thank you!

[Picodes]

@mohammedrizwann123 this specific report is not of Medium severity. It only highlights that one of the project's dependencies is not audited. It is good to be aware of and certainly valuable for users but I don't see how it could fulfill code4rena's definition of a Med severity finding: "Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements."

[0xRizwan]

Hi @Picodes

Thanks for the comment.

Please note this issue has been judged as Medium severity in Tracer audit at code4rena.

Reference: code-423n4/2021-06-tracer-findings#11

While drafting the report, I had referenced it and yes it is very much fulfill Medium severity defination as It is a confirmed finding judged by Code4rena.

I agree some reference is missing in original report but it is highlighted in above response and here and i believe the issue deserves to be particially satisfactory to Medium and made as already duplicate of #423.

Thank you!

[Picodes]

@mohammedrizwann123 the issue you are referring to dates from more than 2 years ago. Please note that referring to another contest is not a valid point and it happens quite frequently that we disagree between judges.

Side note: please recall that the rules are the following: "At the point that a judge provides a response, the conversation is over, unless you wish to provide additional facts which demonstrate inaccurate assumptions in the judge's response. "

[0xRizwan]

@Picodes

No issues, I respect your judging decision.

Thank you!