From d4f8f7527fa4c054761ed1153a7c1ab813d02e8d Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Tue, 6 Aug 2024 22:02:58 +0200 Subject: [PATCH 1/6] Created AUDITS.md, included details from Cantina Finding Created draft version with details of Cantina's review --- audits/AUDITS.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 audits/AUDITS.md diff --git a/audits/AUDITS.md b/audits/AUDITS.md new file mode 100644 index 0000000..5169c8c --- /dev/null +++ b/audits/AUDITS.md @@ -0,0 +1,28 @@ +# Track Record of Audits + +The PRBMath library was included in the Sablier codebase audited by the [Cantina](https://cantina.xyz/welcome) team during May-June of 2023. Likewise, the [Certora](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) team identified a design flaw in July of 2023. Both issues were timely addressed and are now fixed. + +## Cantina Review + +In June-2023, the team realized a security review of `Sablier v1.0.0`, which included `prb-math@v3.3.3"` in the scope. The report of the review included a finding for the PRBMath library, "3.2.3 PRBMath pow() function can return inconsistent values." + +### PRBMath pow() function can return inconsistent values + +_Full report_: [cantina-2023-06-08.pdf](https://github.com/sablier-labs/audits/blob/6567df3fa42b90663e3e694b1e776c6db337a3f2/v2-core/cantina-2023-06-08.pdf) + +_PRBMath reviewed version_: [prb-math@v3.3.3](https://github.com/PaulRBerg/prb-math/tree/v3.3.2) + +_Severity_: High Risk + +_Context_: [sd59x18/Math.sol#L596-L609](https://github.com/PaulRBerg/prb-math/blob/df27d3d12ce12153fb166e1e310c8351210dc7ba/src/sd59x18/Math.sol#L596-L609) + +_Description_: +> PRBMath's pow() function takes in two signed integers, `x` and `y`, and returns `x ** y` +> +> A proper implementation of the pow() function should uphold the following invariant: +> +> `If x >= y and a >= 0, then x ** a >= y ** a.` +> +> This is a crucial invariant for the Sablier protocol, because the pow() function is used by SablierV2LockupDynamic.sol to compute total streamed amounts based on the current time. It is required for the protocol to function as expected that, as block.timestamp increases, the total streamed amount increases as well. Implications of this issue are addressed in C-01 of this report. + +Status: Fixed in https://github.com/PaulRBerg/prb-math/pull/179, which was included in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.0 From f0f2d98841429fbdffcb9b2d86a5ba33d8cf4ab4 Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Tue, 6 Aug 2024 22:57:03 +0200 Subject: [PATCH 2/6] Update AUDITS.md, added Certora's detail Added the details about the security bug made by Certora and refactored the text --- audits/AUDITS.md | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/audits/AUDITS.md b/audits/AUDITS.md index 5169c8c..a3c1c00 100644 --- a/audits/AUDITS.md +++ b/audits/AUDITS.md @@ -1,12 +1,12 @@ -# Track Record of Audits +# Track Record of Security Reviews The PRBMath library was included in the Sablier codebase audited by the [Cantina](https://cantina.xyz/welcome) team during May-June of 2023. Likewise, the [Certora](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) team identified a design flaw in July of 2023. Both issues were timely addressed and are now fixed. ## Cantina Review -In June-2023, the team realized a security review of `Sablier v1.0.0`, which included `prb-math@v3.3.3"` in the scope. The report of the review included a finding for the PRBMath library, "3.2.3 PRBMath pow() function can return inconsistent values." +In June-2023, the team realized a security review of `Sablier v1.0.0`, which included `prb-math@v3.3.3` in the scope. The report of the review included a finding for the PRBMath library: "3.2.3 PRBMath pow() function can return inconsistent values." -### PRBMath pow() function can return inconsistent values +### Issue: PRBMath pow() function can return inconsistent values _Full report_: [cantina-2023-06-08.pdf](https://github.com/sablier-labs/audits/blob/6567df3fa42b90663e3e694b1e776c6db337a3f2/v2-core/cantina-2023-06-08.pdf) @@ -25,4 +25,30 @@ _Description_: > > This is a crucial invariant for the Sablier protocol, because the pow() function is used by SablierV2LockupDynamic.sol to compute total streamed amounts based on the current time. It is required for the protocol to function as expected that, as block.timestamp increases, the total streamed amount increases as well. Implications of this issue are addressed in C-01 of this report. -Status: Fixed in https://github.com/PaulRBerg/prb-math/pull/179, which was included in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.0 +_Status_: `Fixed` in https://github.com/PaulRBerg/prb-math/pull/179, which was included in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.0 + +## Certora Review + +In April-2023, the team reported a security bug in `prb-math@v4.0.0`, the discussion of which was tracked in https://github.com/PaulRBerg/prb-math/discussions/186 + +### Issue: Wrong rounding direction for negative numbers + +_Full Analysis_: [Problems in Solidity Fixed Point Libraries — Certora Bug Disclosure](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) + +_PRBMath reviewed version_: [prb-math@v4.0.0](https://github.com/PaulRBerg/prb-math/tree/v4.0.0) + +_Severity_: Not clear, potentially Critical/High + +_Context_: [src/Common.sol#L387](https://github.com/PaulRBerg/prb-math/blob/7ce3009bbfa0d8e2d430b7a1a9ca46b6e706d90d/src/Common.sol#L387) + +_Description_: +> Roughly speaking, the algorithm PRBMath is using to compute signed multiplication-and-division can be divided into three steps — +> +> 1. Extract the signs and the absolute values of the inputs +> 2. Compute the absolute value of the result by reducing to the unsigned case: +> 3. Compute the overall sign and return the result: +> +> The problem is that step 2 is wrong: Whenever the result is negative, we are actually rounding towards zero and not minus infinity, leading to an off-by-one error. + + +_Status_: `Fixed` with "Clarify rounding modes" in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.1 From 1f6553de59ba1867849db33ad81e09a624e423cc Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Tue, 6 Aug 2024 23:02:03 +0200 Subject: [PATCH 3/6] Updated AUDITS.md --- audits/AUDITS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audits/AUDITS.md b/audits/AUDITS.md index a3c1c00..34a14fc 100644 --- a/audits/AUDITS.md +++ b/audits/AUDITS.md @@ -1,6 +1,6 @@ -# Track Record of Security Reviews +# Security Review History for PRBMath -The PRBMath library was included in the Sablier codebase audited by the [Cantina](https://cantina.xyz/welcome) team during May-June of 2023. Likewise, the [Certora](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) team identified a design flaw in July of 2023. Both issues were timely addressed and are now fixed. +The PRBMath library was included in the Sablier codebase audited by the [Cantina](https://cantina.xyz/welcome) team during May-June of 2023. Likewise, the [Certora](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) team identified a design flaw in April of 2023. Both issues were timely addressed and are now fixed. ## Cantina Review From 5094ba6ce503464c1d2fa7f6f0cc534a189dd1df Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:18:01 +0200 Subject: [PATCH 4/6] Updated AUDITS.md --- audits/AUDITS.md | 59 ++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 39 deletions(-) diff --git a/audits/AUDITS.md b/audits/AUDITS.md index 34a14fc..89df1ec 100644 --- a/audits/AUDITS.md +++ b/audits/AUDITS.md @@ -1,54 +1,35 @@ # Security Review History for PRBMath +| :warning: | Audits are not a guarantee of correctness. Some parts of the code base were modified after they were audited. | +| --------- | :------------------------------------------------------------------------------------------------------------ | + The PRBMath library was included in the Sablier codebase audited by the [Cantina](https://cantina.xyz/welcome) team during May-June of 2023. Likewise, the [Certora](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) team identified a design flaw in April of 2023. Both issues were timely addressed and are now fixed. +| Auditor | Type | Initial Commit | Report | +| :---------------- | :------ | :------------------- | :--------------------------------------------- | +| Cantina | Firm | [prb-math@v3.3.3](https://github.com/PaulRBerg/prb-math/tree/v3.3.2) | [2023-06-08](https://github.com/sablier-labs/audits/blob/6567df3fa42b90663e3e694b1e776c6db337a3f2/v2-core/cantina-2023-06-08.pdf) | +| Certora | Firm | [prb-math@v4.0.0](https://github.com/PaulRBerg/prb-math/tree/v4.0.0) | [2023-07-12](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) | + ## Cantina Review -In June-2023, the team realized a security review of `Sablier v1.0.0`, which included `prb-math@v3.3.3` in the scope. The report of the review included a finding for the PRBMath library: "3.2.3 PRBMath pow() function can return inconsistent values." +In May-June of 2023, the team realized a security review of `Sablier v1.0.0`, which included `prb-math@v3.3.3` in the scope. The report of the review included a finding for the PRBMath library: "3.2.3 PRBMath pow() function can return inconsistent values." ### Issue: PRBMath pow() function can return inconsistent values -_Full report_: [cantina-2023-06-08.pdf](https://github.com/sablier-labs/audits/blob/6567df3fa42b90663e3e694b1e776c6db337a3f2/v2-core/cantina-2023-06-08.pdf) - -_PRBMath reviewed version_: [prb-math@v3.3.3](https://github.com/PaulRBerg/prb-math/tree/v3.3.2) - -_Severity_: High Risk - -_Context_: [sd59x18/Math.sol#L596-L609](https://github.com/PaulRBerg/prb-math/blob/df27d3d12ce12153fb166e1e310c8351210dc7ba/src/sd59x18/Math.sol#L596-L609) - -_Description_: -> PRBMath's pow() function takes in two signed integers, `x` and `y`, and returns `x ** y` -> -> A proper implementation of the pow() function should uphold the following invariant: -> -> `If x >= y and a >= 0, then x ** a >= y ** a.` -> -> This is a crucial invariant for the Sablier protocol, because the pow() function is used by SablierV2LockupDynamic.sol to compute total streamed amounts based on the current time. It is required for the protocol to function as expected that, as block.timestamp increases, the total streamed amount increases as well. Implications of this issue are addressed in C-01 of this report. - -_Status_: `Fixed` in https://github.com/PaulRBerg/prb-math/pull/179, which was included in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.0 +- _Full report_: [cantina-2023-06-08.pdf](https://github.com/sablier-labs/audits/blob/6567df3fa42b90663e3e694b1e776c6db337a3f2/v2-core/cantina-2023-06-08.pdf) +- _PRBMath version_: [prb-math@v3.3.3](https://github.com/PaulRBerg/prb-math/tree/v3.3.2) +- _Severity_: High Risk +- _Context_: [sd59x18/Math.sol#L596-L609](https://github.com/PaulRBerg/prb-math/blob/df27d3d12ce12153fb166e1e310c8351210dc7ba/src/sd59x18/Math.sol#L596-L609) +- _Status_: `Fixed` in https://github.com/PaulRBerg/prb-math/pull/179, which was included in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.0 ## Certora Review -In April-2023, the team reported a security bug in `prb-math@v4.0.0`, the discussion of which was tracked in https://github.com/PaulRBerg/prb-math/discussions/186 +In April of 2023, the team reported a security bug in `prb-math@v4.0.0`, the discussion of which was tracked in https://github.com/PaulRBerg/prb-math/discussions/186 ### Issue: Wrong rounding direction for negative numbers -_Full Analysis_: [Problems in Solidity Fixed Point Libraries — Certora Bug Disclosure](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) - -_PRBMath reviewed version_: [prb-math@v4.0.0](https://github.com/PaulRBerg/prb-math/tree/v4.0.0) - -_Severity_: Not clear, potentially Critical/High - -_Context_: [src/Common.sol#L387](https://github.com/PaulRBerg/prb-math/blob/7ce3009bbfa0d8e2d430b7a1a9ca46b6e706d90d/src/Common.sol#L387) - -_Description_: -> Roughly speaking, the algorithm PRBMath is using to compute signed multiplication-and-division can be divided into three steps — -> -> 1. Extract the signs and the absolute values of the inputs -> 2. Compute the absolute value of the result by reducing to the unsigned case: -> 3. Compute the overall sign and return the result: -> -> The problem is that step 2 is wrong: Whenever the result is negative, we are actually rounding towards zero and not minus infinity, leading to an off-by-one error. - - -_Status_: `Fixed` with "Clarify rounding modes" in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.1 +- _Full Analysis_: [Problems in Solidity Fixed Point Libraries — Certora Bug Disclosure](https://medium.com/certora/problems-in-solidity-fixed-point-libraries-certora-bug-disclosure-987f504daca4) +- _PRBMath version_: [prb-math@v4.0.0](https://github.com/PaulRBerg/prb-math/tree/v4.0.0) +- _Severity_: Not clear, potentially Critical/High +- _Context_: [src/Common.sol#L387](https://github.com/PaulRBerg/prb-math/blob/7ce3009bbfa0d8e2d430b7a1a9ca46b6e706d90d/src/Common.sol#L387) +- _Status_: `Fixed` with "Clarify rounding modes" in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.1 From 0ef616b69f84bf6f1513636a56efd8ee85938880 Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:22:03 +0200 Subject: [PATCH 5/6] Updated README.md --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 7618744..448a67c 100644 --- a/README.md +++ b/README.md @@ -392,8 +392,7 @@ You will need the following VSCode extensions: ## 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. +The codebase has undergone rigorous audits by leading security experts from Cantina and Certora. For a comprehensive list of all audits conducted, please click [here](https://github.com/PaulRBerg/prb-math/blob/main/audits/AUDITS.md). ### Caveat Emptor From 1e0240f74621b725ea4f0282b7a78efb5b782431 Mon Sep 17 00:00:00 2001 From: JP <89362795+0xJayPi@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:24:39 +0200 Subject: [PATCH 6/6] Updated AUDITS.md --- audits/AUDITS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/audits/AUDITS.md b/audits/AUDITS.md index 89df1ec..6ea7bd6 100644 --- a/audits/AUDITS.md +++ b/audits/AUDITS.md @@ -32,4 +32,5 @@ In April of 2023, the team reported a security bug in `prb-math@v4.0.0`, the dis - _PRBMath version_: [prb-math@v4.0.0](https://github.com/PaulRBerg/prb-math/tree/v4.0.0) - _Severity_: Not clear, potentially Critical/High - _Context_: [src/Common.sol#L387](https://github.com/PaulRBerg/prb-math/blob/7ce3009bbfa0d8e2d430b7a1a9ca46b6e706d90d/src/Common.sol#L387) -- _Status_: `Fixed` with "Clarify rounding modes" in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.1 +- _Status_: `Mitigated` with "Clarify rounding modes" in release https://github.com/PaulRBerg/prb-math/releases/tag/v4.0.1 +