Security Audit
Nov 13, 2024
Version 1.0.0
Presented by 0xMacro
This document includes the results of the security audit for Seven Seas's program code as found in the section titled ‘Source Code’. The security audit was performed by the Macro security team from Nov 6th to Nov 12th 2024.
The purpose of this audit is to review the source code of certain Seven Seas Solana programs, and provide feedback on the design, architecture, and quality of the source code with an emphasis on validating the correctness and security of the software in its entirety.
Disclaimer: While Macro’s review is comprehensive and has surfaced some changes that should be made to the source code, this audit should not solely be relied upon for security, as no single audit is guaranteed to catch all possible bugs.
The following is an aggregation of issues found by the Macro Audit team:
Severity | Count | Acknowledged | Won't Do | Addressed |
---|---|---|---|---|
Low | 1 | 1 | - | - |
Code Quality | 4 | - | - | 4 |
Seven Seas was quick to respond to these issues.
Our understanding of the specification was based on the following sources:
The following source code was reviewed during the audit:
5239c754dc0730216def82997d41560623a2216d
Specifically, we audited Eclipse programs that allow bridging token-2022 from Eclipse chain to Ethereum chain using Hyperlane.
Source Code | SHA256 |
---|---|
programs/boring-bridge-holder/src/events.rs |
|
programs/boring-bridge-holder/src/lib.rs |
|
programs/boring-bridge-holder/src/instructions/mod.rs |
|
programs/boring-bridge-holder/src/instructions/transfer_remote.rs |
|
Note: This document contains an audit solely of the Solana programs listed above. Specifically, the audit pertains only to the programs themselves, and does not pertain to any other programs or scripts, including deployment scripts.
Click on an issue to jump to it, or scroll down to see them all.
We quantify issues in three parts:
This third part – the severity level – is a summary of how much consideration the client should give to fixing the issue. We assign severity according to the table of guidelines below:
Severity | Description |
---|---|
(C-x) Critical |
We recommend the client must fix the issue, no matter what, because not fixing would mean significant funds/assets WILL be lost. |
(H-x) High |
We recommend the client must address the issue, no matter what, because not fixing would be very bad, or some funds/assets will be lost, or the code’s behavior is against the provided spec. |
(M-x) Medium |
We recommend the client to seriously consider fixing the issue, as the implications of not fixing the issue are severe enough to impact the project significantly, albiet not in an existential manner. |
(L-x) Low |
The risk is small, unlikely, or may not relevant to the project in a meaningful way. Whether or not the project wants to develop a fix is up to the goals and needs of the project. |
(Q-x) Code Quality |
The issue identified does not pose any obvious risk, but fixing could improve overall code quality, on-chain composability, developer ergonomics, or even certain aspects of protocol design. |
(I-x) Informational |
Warnings and things to keep in mind when operating the protocol. No immediate action required. |
(G-x) Gas Optimizations |
The presented optimization suggestion would save an amount of gas significant enough, in our opinion, to be worth the development cost of implementing it. |
evm_recipient
can lead to the funds being stuck in the bridge forever
In order to bridge tokens using Hyperlane protocol, the input destination recipient address must be 32 bytes to generalize the case where the destination chain is not an EVM-compatible chain.
Since the Boring Bridge Holder intends to bridge only from the Eclipse chain to the ETH chain, it’s unnecessary to store the destination recipient address in 32 bytes. The evm_recipient
configuration in the program currently has [u8; 32]
type.
However, 32 bytes allows for a potentially malformed address. If a value has any data in the 12 leftmost bytes, this will cause the transaction to fail on the ETH chain (specifically, Hyperlane’s TypeCasts helper), leaving the funds stuck in the bridge indefinitely.
Remediations to Consider
Consider changing evm_recipient
type to [u8; 20]
type, then converting to [u8; 32]
type when cross-program invocating to the Hyperlane protocol.
We recognize that having the 32 byte evm recipient could lead to stuck funds if an improper EVM address is supplied. To mitigate this we will throughly test the set configuration to confirm that it does in fact bridge assets back to the proper EVM address. Because of the config hash setup if the strategist were to then pass in a malformed evm address, the transaction would revert from the generated hash being different from the one stored in the BoringState account
Currently, when initializing a new boring account, the account size is manually calculated based on the size of the BoringState
struct. Rather than calculating this ourselves, we can use the std::mem::size_of
utility to determine the size of the struct we are trying to store:
+ use std::mem::size_of;
...
pub struct Initialize<'info> {
#[account(
init,
payer = signer,
- space = 8 + 32 + 32 + 32 + 32 + 1,
+ space = typeof::<BoringState>() + 8,
seeds = [b"boring_state", signer.key().as_ref()],
bump
)]
pub boring_account: Account<'info, BoringState>,
#[account(mut)]
pub signer: Signer<'info>,
pub system_program: Program<'info, System>,
}
Reference: src/lib.rs#L252
#[account()]
field-level attributes
Throughout the program, some struct fields use the #[account()]
attributes with empty parentheses, which adds no logical functionality. Consider removing these #[account()]
attributes.
#[account(mut)]
field-level attributes
In the UpdateOwner
, UpdateStrategist
, and UpdateConfiguration
struct, there are struct fields called signer
using #[account(mut)]
attributes.
Generally,#[account(mut)]
field-level attribute used in a Signer
type field is when that signer is spending or receiving lamports (not including gas payment) within the transaction. In our case, transfer_ownership()
, update_strategist()
, and update_configuration()
functions will not affect the caller’s balance; hence, we can safely remove the #[account(mut)]
attributes from those struct fields.
BorshDeserialize
derived attribute
Topic | Best practices |
---|---|
Impact | Low |
The TransferRemote
struct includes BorshDeserialize
type attribute, which is not used anywhere. Consider removing the attribute
- #[derive(BorshSerialize, BorshDeserialize, Debug, Clone)]
+ #[derive(BorshSerialize, Debug, Clone)]
pub struct TransferRemote {
pub destination_domain: u32,
pub recipient: [u8; 32],
pub amount_or_id: [u8; 32],
}
Reference: lib.rs#L413-L418
Macro makes no warranties, either express, implied, statutory, or otherwise, with respect to the services or deliverables provided in this report, and Macro specifically disclaims all implied warranties of merchantability, fitness for a particular purpose, noninfringement and those arising from a course of dealing, usage or trade with respect thereto, and all such warranties are hereby excluded to the fullest extent permitted by law.
Macro will not be liable for any lost profits, business, contracts, revenue, goodwill, production, anticipated savings, loss of data, or costs of procurement of substitute goods or services or for any claim or demand by any other party. In no event will Macro be liable for consequential, incidental, special, indirect, or exemplary damages arising out of this agreement or any work statement, however caused and (to the fullest extent permitted by law) under any theory of liability (including negligence), even if Macro has been advised of the possibility of such damages.
The scope of this report and review is limited to a review of only the code presented by the Seven Seas team and only the source code Macro notes as being within the scope of Macro’s review within this report. This report does not include an audit of the deployment scripts used to deploy the Solana programs in the repository corresponding to this audit. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. In this report you may through hypertext or other computer links, gain access to websites operated by persons other than Macro. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such websites’ owners. You agree that Macro is not responsible for the content or operation of such websites, and that Macro shall have no liability to your or any other person or entity for the use of third party websites. Macro assumes no responsibility for the use of third party software and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.