Skip to content

SevenSeas-41 report #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SevenSeas-41 report #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions client/library/library/audits/sevenSeas-41.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<page
clientName="Seven Seas"
reportDate="May 5, 2025"
auditTitle="Seven Seas A-41"
auditVersion="1.0.0"
repoUrl="https://github.com/Veda-Labs/boring-vault"
layout="/library/audits/_layout.html"
customRepoInfo
>

<content-for name="schedule">
The security audit was performed by the Macro security team over two days starting on April 25th, 2025.
</content-for>

<content-for name="spec">
<ul>
<li>Discussions with the {{page.clientName}} team.</li>
<li>Available documentation in the repository.</li>
</ul>
</content-for>

<content-for name="repo-info">
<ul>

<li class="break-words break-all">
<b>Mellow's dvStETH vault decoder (from <a href="https://github.com/Veda-Labs/boring-vault/pull/251">PR 251</a>)</b> </br>
Commit Hash: <code>478f3a4a1457fd2dd296fc48d43ad5689998cc4a</code>
<template type="file-hashes">
38baba1ff4fbc624b2287956735fab8c1cef679b588c0ac6f0b1b72f92c6db41 src/base/DecodersAndSanitizers/Protocols/DvStETHDecoderAndSanitizer.sol
</template>
</li>

<li class="break-words break-all">
<b>Generic rate provider with decimal scaling (from <a href="https://github.com/Veda-Labs/boring-vault/pull/254">PR 254</a>)</b> </br>
Commit Hash: <code>63b8a392581db533ad64a5371bef3a17d6a746d7</code>
<template type="file-hashes">
b5fa65ad59eb27e216e88b294be5909db2f9775b09a77de4bd295190864add26 src/helper/GenericRateProviderWithDecimalScaling.sol
</template>
</li>

<li class="break-words break-all">
<b>Teller Decoder fix (from <a href="https://github.com/Veda-Labs/boring-vault/pull/261">PR 261</a>)</b> </br>
Commit Hash: <code>75a57946ea4d4f8416c34bc18ab13c42f07ee3de</code>
<template type="file-hashes">
a60d2e60d92d27c3b9eb28438862f2b7f21e0196c6560f7fea2eed13a50f2b00 src/base/DecodersAndSanitizers/Protocols/TellerDecoderAndSanitizer.sol
</template>
</li>

<li class="break-words break-all">
<b>Permissioned transfers for teller (from <a href="https://github.com/Veda-Labs/boring-vault/pull/256">PR 256</a>)</b> </br>
Commit Hash: <code>b719f64af56f49781ca74604ec50ea7927818aa2</code>
<template type="file-hashes">
b4f6b373696ce73a739901bc5b1d118b5d83ccd9282b840a08ef25b9c9a1647b src/base/Roles/TellerWithMultiAssetSupport.sol
</template>
</li>

<li class="break-words break-all">
<b>Beraborrow decoder (from <a href="https://github.com/Veda-Labs/boring-vault/pull/258">PR 258</a>)</b> </br>
Commit Hash: <code>e70b91e6173ac13d15038c05473bc5b0a8d47a1c</code>
<template type="file-hashes">
f57f1547b5472b8fdc12f4358829ffd3316a0804c6aa58e20cab5ed216e6ad36 src/base/DecodersAndSanitizers/Protocols/BeraborrowDecoderAndSanitizer.sol
</template>
</li>
</ul>
</content-for>
</page>
58 changes: 58 additions & 0 deletions content/collections/public/sevenSeas-41-issues.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<item>
<field name="topic">Validation</field>
<field name="impact">medium</field>
<field name="chance">medium</field>
<field name="status">fixed</field>
<field name="customCommits">https://github.com/Veda-Labs/boring-vault/pull/261/commits/f6b0f74c7c3cad860988a204bc6c77224087b803,https://github.com/Veda-Labs/boring-vault/pull/261/commits/9ed139937687f35b6c3977d09f9957bbcbdf0f81</field>
<field name="content">
## [M-1] Incorrect sanitization of destination chain

The assembly blocks in both the merkle tree generation and the decoder's `depositAndBridge` function aim to convert a `uint32` value from a `bytes` array to an `address` type. However, due to the final right shift operation (`shr(96, word)`), the conversion always returns `0x0` for the chain ID. The test appears to pass only because both functions return `0x0` for the address, meaning the sanitization is ineffective and allows any chain ID to be passed.

**Remediation to Consider**

Remove the right shift operation from the assembly blocks to preserve the actual converted value.
</field>
</item>

<item>
<field name="topic">Validation</field>
<field name="impact">medium</field>
<field name="chance">low</field>
<field name="status">fixed</field>
<field name="commit">b9809e67e13bd25f18bf2ecfd612ddd5defd84d1</field>
<field name="content">
## [L-1] Mellow decoder doesn’t sanitize the deposit token

In Mellow's [deposit function](https://github.com/Veda-Labs/boring-vault/pull/251/files#diff-e2a9ee842351c79873643646c5c510da1355cd6b49d7ecac8bb4ac86beebc63aR20), the parameter `amounts` determines which underlying vault tokens are being deposited based on their index in the array. While currently only `WETH` and `wstETH` tokens are supported, protocol admins can add additional tokens through the Vault's `addToken` function. To maintain control over which tokens can be deposited, the boring vault needs to sanitize the underlying tokens based on their index in the `amounts` array.
The likelihood of this issue is low since token deposits are additionally restricted by token approvals.

**Remediation to Consider**

Retrieve the vault's underlying tokens through the `underlyingTokens()` function and sanitize them according to their corresponding positions in the `amounts` array.
</field>
</item>


<item>
<field name="topic">Validation</field>
<field name="impact">medium</field>
<field name="chance">low</field>
<field name="status">fixed</field>
<field name="commit">d739d7e1c0d052ef54edc45c0e63714fe378e828</field>
<field name="content">
## [L-2] `preDeposits` can be passed in `adjustDenVault`

In the Beraborrow decoder's `openDenVault` function, passing `preDeposit` is explicitly disallowed by this check:

```solidity
if (params._preDeposit.length > 0) revert BeraborrowDecoderAndSanitizer__PredepositLengthGtZero();
```

However, the `adjustDenVault` function lacks this check, which allows values to be passed to the `_preDeposit` parameter. This could enable deposits of assets not explicitly allowed by the boring vault.

**Remediation to Consider**

Add the above check to the `adjustDenVault` function.
</field>
</item>