- November 10, 2025
OpenZeppelin Security
OpenZeppelin Security
Security Audits
Summary
Type: DeFi
Timeline: January 21, 2025 → January 28, 2025
Languages: Golang
Findings
Total issues: 18 (16 resolved)
Critical: 0 (0 resolved)
High: 0 (0 resolved)
Medium: 0 (0 resolved)
Low: 4 (4 resolved)
Notes & Additional Information
13 (11 resolved)
Scope
We audited the lombard-finance/ledger repository at commit b52bbdb. Certain files in scope, outlined below, were audited in full, while the others were diff-audited against commit 60f439d.
In scope for the full audit were the following files:
ledger
├── notaryd/blacklist
│ ├── evm.go
│ └── service.go
└── notaryd/verifier/sui
├── config.go
└── fetcher.go
In scope for the diff audit were the following files:
ledger
├── notaryd
│ ├── config/config.go
│ ├── events/source.go
│ ├── notary/notarization.go
│ ├── notary/registration.go
│ ├── types/deposit_btc_msg.go
│ ├── verifier/deriveaddress/service.go
│ ├── verifier/deposit_bridge_strategy.go
│ ├── verifier/deposit_strategy.go
│ ├── verifier/unstake_strategy.go
│ ├── verifier/verifier.go
│ └── start.go
├── utils/broadcast/broadcast.go
└── x
├── deposit
│ ├── exported/destination_chain_id.go
│ ├── exported/lbtc_address.go
│ ├── keeper/query_derive_address.go
│ ├── module/autocli.go
│ └── types/errors.go
└── notary
│ ├── module/autocli.go
│ ├── keeper/query_get_notary_session.go
│ └── keeper/query_get_notary.go
System Overview
Lombard bridges the gap between Bitcoin staking and DeFi by enabling BTC holders to provide economic security to DeFi protocols. At its core, the protocol allows users to stake Bitcoin through Babylon and receive LBTC, which can then be used to provide liquidity across multiple blockchain networks. Babylon is a protocol that extends Bitcoin's utility by allowing it to secure Proof-of-Stake networks and applications. While Babylon enables Bitcoin staking, it requires locking up the BTC, which limits its broader utility. Lombard addresses this limitation through LBTC, creating a liquid representation of Babylon-staked positions that remain active in DeFi markets.
When users interact with Lombard, they deposit Bitcoin into Lombard addresses which is then staked through Babylon to generate rewards. Users receive LBTC tokens on their chosen blockchain at a 1:1 ratio with their deposited BTC. These tokens can be used in DeFi applications while continuing to earn staking rewards. Users can later redeem their LBTC for BTC by burning the tokens, which triggers the release of the original BTC deposit from the Lombard address. They can also transfer LBTC between supported chains through either a canonical bridge or, on certain chains, through the Layer Zero Omnichain ERC-20 bridge.
Prior to this audit, the Ledger system only supported EVM-compatible destination chains. This audit introduced modifications to the Ledger code to enable support for depositing BTC and minting LBTC on the Sui blockchain, as well as unstaking from Sui to receive BTC on the Bitcoin blockchain. At the time of the audit, transferring LBTC from the Sui blockchain to other blockchains was not yet implemented. Additionally, this update introduced a blacklist mechanism for the Bitcoin blockchain, designed to flag specific UTXOs and prevent blacklisted deposits from being minted on a destination chain.
The scope of this audit was limited to the changes made to the Ledger codebase (the Cosmos portion). Therefore, various assumptions were made about the development and functionality of the Sui blockchain's smart contracts and their interaction with the Ledger blockchain.
Security Model and Trust Assumptions
This audit focused on the addition of new features to Lombard's product, with the security model and trust assumptions remaining consistent with those established in the prior audit. The new blacklist feature maintains a list of blacklisted UTXOs on Bitcoin, and it is trusted that the party updating this list acts in the best interest of the protocol.
Furthermore, as previously mentioned, the changes made to support the Sui blockchain as a destination chain were only audited from the Cosmos side. Therefore, it was assumed that the smart contract code on the Sui blockchain would function equivalently to the implementations on EVM-compatible blockchains.
Low Severity
Lack of Typed Addresses and Inconsistent Validation
The current implementation handles Sui and EVM addresses as generic []byte entities with inconsistent validation logic between the two. Sui addresses have specific checks, such as trimming the "0x" prefix and verifying the length while EVM addresses do not undergo similar validation. This inconsistency increases the likelihood of bugs and makes it harder to enforce domain-specific rules or distinguish between different address types, increasing the risk of mixing incompatible formats.
Consider introducing dedicated types for Sui and EVM addresses with associated methods for validation and normalization. This ensures consistency in address handling, reduces duplication of logic, and improves type safety.
Update: Resolved in pull request #162 at commit 9dcb4f5.
Lack of a Strongly Typed Chain ID Structure Increases Risk of Misuse
The current implementation uses []byte to represent destination chain IDs which are specialized identifiers combining the canonical chain ID of a blockchain with its ecosystem (e.g., EVM, Solana, and Cosmos). Treating these identifiers as generic byte arrays obscures their meaning, complicates the enforcement of proper validation, and increases the risk of misuse.
Consider defining a dedicated struct for chain IDs and providing methods for validation, serialization, and comparison. This approach enhances type safety, reduces ambiguity, and improves the maintainability of the codebase. In addition, consider renaming the struct to something more explicit, such as ChainUID or ChainHash, to avoid confusion with the canonical chain ID.
Update: Resolved in pull request #176 at commit 6cf5bbe. The team stated:
A ChainId interface along with its specialized types has been defined and moved to a shared repository https://github.com/lombard-finance/chain
NewDestinationChainId Silently Truncates or Zero-Pads Input
The NewDestinationChainId function allocates a fixed 32-byte array for every input, regardless of its length. If the input is shorter than 32 bytes, the remaining bytes are zero-filled. If the input is longer than 32 bytes, it is silently truncated. This behavior introduces ambiguity and risks potential misbehavior as the function does not validate the input length before processing it.
Consider validating the input length before the allocation to ensure that it matches the expected size. If the length is incorrect, return an error to prevent unexpected behavior and reduce the risk of bugs caused by malformed DestinationChainIds.
Update: Resolved in pull request #176. The team stated:
The chain id code has been moved to a shared repository.
Missing ethclient.Client Reference in EvmBlacklist
The EvmBlacklist struct does not store the ethclient.Client instance created by ethclient.Dial during initialization. As a result, the connection cannot be explicitly closed when it is no longer needed, leading to potential resource leaks. Furthermore, if the connection to the Ethereum RPC node drops, the IsBlacklisted calls to the contract will fail without a mechanism to re-establish the connection.
Consider storing ethclient.Client in the EvmBlacklist struct and adding reconnection logic for the case when the connection drops.
Update: Resolved in pull request #163 at commit c3b5adc. The Lombard team stated:
Added a close for resources cleanup. Reconnection logic is already handled internally by rpc.Client from the go-ethereum package.
Notes & Additional Information
Inconsistent Initialization of createVerifier Services
The createVerifier function initializes services using the cfg object, but it handles the configuration data inconsistently. The sanction.NewService function accepts the SanctionsConfig as a copy, while the blacklist.NewEvmBlacklist function accepts the BlacklistConfig as a pointer.
Consider standardizing the method of passing configuration data to services by consistently passing either a copy or a pointer, based on the intended immutability of the configuration.
Update: Resolved in pull request #173 at commit bc68227.
Lack of Stack Trace in Error Wrapping
The current error-handling implementation wraps errors using contextual strings but does not include a stack trace. An example of this is in this line of code which can return an error inside another error. While this approach is idiomatic in Go, this can result in excessively long chained error strings, making it harder to pinpoint the source of the issue in this large and multi-layered codebase.
Consider adopting an error-handling strategy that includes more detailed contextual information, such as stack traces or structured error metadata, to facilitate efficient debugging.
Update: Acknowledged, not resolved. The Lombard team stated:
Thanks for the comment. Currently the codebase adopts the principle of constructing errors by adding contextual information at each new wrapping, until the error is either managed or reported to the caller/user. In this way the more the error propagates in the call stack, the more information it carries, despite being unstructured in the error string. As you report, such pattern is idiomatic in go. At the same time, whenever a panic occurs, the runtime stack trace is dumped automatically dumped to logs, which helps in investigating the potential issues. For these reasons, in absence of a specific flow, we will keep the codebase unchanged in respect to this issue.
Duplicated String and Hex Functions
The DestinationChainId type defines String and Hex functions, both performing the same operation of returning the hex-encoded representation as a string. This redundancy increases maintenance complexity and risks inconsistencies if the logic changes.
Consider consolidating the functionality into one function to reduce duplication. Alternatively, consider having one function call the other if both are needed for interface compatibility.
Update: Resolved in pull request #176 at commit 06ef89e. The Lombard team stated:
The chain id code has been moved to a shared repository https://github.com/lombard-finance/chain. Now String() calls Hex() to avoid duplication.
Missing Length Check for chainId in Fetcher GetUnstake
The GetUnstake function assumes that the chainId parameter is at least SUI_CHAINID_LENGTH bytes long. This would cause an unhandled panic if the length of chainId were less than SUI_CHAINID_LENGTH, as it attempts to access an invalid slice range.
Consider adding a safety check to validate the length of chainId before attempting to slice it. If the length is insufficient, the function should return an error to prevent a panic.
Update: Resolved in pull request #176 at commit 06ef89e. The Lombard team stated:
The chain id code has been moved to a shared repository https://github.com/lombard-finance/chain . All chain ids are checked within the new defined type.
Missing Nil Checks in newDepositStrategy May Lead to Runtime Panic
The newDepositStrategy function does not validate that its input references are not nil. If a nil reference is passed for btcFetcher, sanctions, depositCli, or blacklist, the function will initialize successfully but will fail at runtime when attempting to access these values. This behavior increases the risk of runtime panics and makes debugging more difficult, especially in scenarios where the issue originates from an unexpected nil input.
Consider adding explicit checks for nil values at the beginning of the newDepositStrategy function. If any of the references are nil, return an error or log a message to prevent unintentional usage.
Update: Acknowledged, not resolved. The Lombard team stated:
Thanks for the comment. However, all the variable passed to such strategy are checked to be non-nil when created. The
newDepositStrategy, and the analogous functions of the other verification strategies, are not exported outside the package, there is not other usage than the creation of the strategy in the only place where they are called, and there is not any external input directly reported to such function. So, we think there is not any need to recheck such references.
Validate Address Earlier in NormalizedHexAddress
In the NormalizedHexAddress function, the msg.Address length is validated only after attempting to create a DestinationChainId from msg.ChainId. If msg.Address has an invalid length, the function could unnecessarily proceed to process the chain ID before returning an error.
Consider validating the address length at the beginning of the function prior to any other processing.
Update: Resolved in pull request #165 at commit d572625.
nodeClient Connection Not Properly Closed
The listen function initializes a nodeClient connection but does not ensure that the connection is properly closed. While the nodeClient is started and used throughout the function, no Stop() call is made to terminate the connection when the function exits. This can result in resource leaks or hanging connections, especially in long-running processes or when the function is invoked multiple times.
Consider using a defer statement or explicitly calling nodeClient.Stop() during the function's cleanup process. This ensures that the connection is gracefully closed regardless of whether the function exits normally or due to an error.
Update: Resolved in pull request #166.
Use of iota May Lead to Unintended Errors
In destination_chain_id.go, the Go iota keyword is used to define three consecutive constants. While iota can simplify the declaration of sequential constants, it is error-prone as unintended modifications, such as reordering or adding new constants, can inadvertently alter the values assigned to existing constants.
Consider explicitly assigning constant values during their definition to make the codebase more resilient to changes and to improve readability and maintainability.
Update: Resolved in pull request #176 at commit 06ef89e.
Confusing Naming for DepositBtcAddressLength
To favor explicitness and readability, the DepositBtcAddressLength constant may benefit from a more descriptive name. The current name is misleading, as it does not represent the BTC address length, but rather the length of the destination chain address retrieved from the DepositBtcMsg struct.
Consider renaming the DepositBtcAddressLength constant to DepositBtcMsgAddressLength or DestinationAddressLength to better reflect its purpose.
Update: Resolved in pull request #167 at commit 7f3b9c6.
Typographical Errors
Throughout the codebase, multiple instances of typographical errors were identified:
- This error message says that "gottend tx id value mismatches the expected one", but it should be updated to say "on-chain tx id value mismatches the expected one" instead.
- This error message says "goint to activate", but it should be updated to say "going to activate" instead.
- This parameter in the
NotarydConfigstruct is namedInitiaLedgerReconnections, but it should be namedInitialLedgerReconnectionsinstead.
To improve the clarity of the codebase, consider correcting the above and any other instances of typographical errors.
Update: Resolved in pull request #174.
Unnecessary Variable Return
In the NewEvmBlacklist function in evm.go, the return statement in line 38 returns the bl, err variables. However, the value of err is already checked to be nil immediately before this return statement.
Consider updating the return statement to explicitly return bl, nil instead of bl, err for improved clarity.
Update: Resolved in pull request #168 at commit 5658e65.
Unused Parameter
In the handleErrors function in source.go, the logger parameter is never used in the function definition.
Consider removing this parameter for improved code clarity and conciseness.
Update: Resolved in pull request #171 at commit 9778c85.
Non-Constant Format String in Call
In this error message in query_get_notary_session.go, Wrapf receives a non-constant format string.
Consider including "%s" inside the call to ensure that the format string is constant.
Update: Resolved in pull request #172 at commit f9eba38.
Client Reported
Upgrades to the LBTC SUI Contract Break Unstake Verification
In the Sui ecosystem, every deployed smart contract package is assigned a unique package ID. Although Sui supports upgradability by default, any upgrade results in a new package ID. Because the LBTC unstake verification process depends on checking the LBTC package ID, upgrading the LBTC contract would break the unstake verification.
Simply updating the package ID in the ledger each time the LBTC contract is upgraded is not a viable solution as it would disrupt BTC deposit addresses. This is because the derivation of these addresses uses the LBTC contract address as a salt in its derivation, so modifying the package ID would result in a completely different set of deposit addresses for the same user input.
Update: Resolved in pull request #198 at commits d2d5a32, 901606b, 48f8b59, d8c32f7, 04f3774 and 60e6784. The Lombard team resolved this issue by parsing the LBTC contract event, which retains the original package ID even after an upgrade, ensuring that the unstake verification process remains functional without the need to keep track of the latest package ID.
Conclusion
Minor code recommendations were made to Lombard's ledger codebase, particularly related to code conciseness, best practices, and documentation. As previously stated, as the protocol matures, transitioning the current permissioned Consortium system toward an economically secure model will be crucial for long-term protocol safety.
The Lombard team was highly responsive throughout the audit process and worked diligently to address the identified issues.
Ready to secure your code?