Starknet Integration Audit

Summary

Type: Cross-Chain
Timeline: May 20, 2025 → May 23, 2025
Languages: Go

Findings
Total issues: 9 (9 resolved)
Critical: 0 (0 resolved)
High: 0 (0 resolved)
Medium: 2 (2 resolved)
Low: 4 (4 resolved)

Notes & Additional Information
3 (3 resolved)

Scope

OpenZeppelin performed a differential audit of pull request #284 of the lombard-finance/ledger repository at commit 2ed1e11 against commit 29d1b5b.

In scope were the following files:

 ledger
├── notaryd
│   ├── config
│   │   └── config.go
│   ├── start.go
│   ├── types
│   │   └── unstake_msg.go
│   └── verifier
│       ├── deposit_strategy.go
│       ├── starknet
│       │   ├── config.go
│       │   └── fetcher.go
│       ├── unstake_strategy.go
│       └── verifier.go
├── go.mod
└── go.sum

System Overview

Lombard bridges the gap between Bitcoin (BTC) staking and DeFi by enabling BTC holders to provide economic security to DeFi protocols. At its core, the protocol allows users to stake BTC through Babylon and receive LBTC, which can then be used to provide liquidity across multiple blockchain networks. Babylon is a protocol that extends BTC's utility by allowing it to secure Proof-of-Stake networks and applications. While Babylon enables BTC 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 BTC 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 continuously earning 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 across supported chains through either a canonical bridge or, on certain chains, through the LayerZero Omnichain ERC-20 bridge.

The audited codebase introduces modifications to the Ledger and the companion service responsible for notarization, notaryd, which allows for depositing BTC and minting LBTC on the Starknet blockchain, as well as unstaking from Starknet to receive BTC on the Bitcoin blockchain. However, transferring LBTC from Starknet to other blockchains is not supported.

The scope of this audit was limited to the changes made to the Ledger codebase. Therefore, various assumptions were made about the development and functionality of the Starknet blockchain's smart contracts and their interaction with the Ledger blockchain.

Security Model and Trust Assumptions

This audit focused on the addition of the aforementioned new features to the Ledger, with the security model and trust assumptions remaining consistent with those established in the initial Lombard audit. The changes made to support the Starknet blockchain as a destination chain were only audited from the Ledger side. Therefore, it was assumed that the Starknet programs would function equivalently to the implementation of the EVM smart contracts.

The notaryd companion service does not prevent duplicate deposit and unstake payloads from being notarized twice. Thus, it is up to the consumer (i.e., the payload spender) to prevent double-spending. Hence, for deposits, the Starknet program, and all other relevant contracts, should ensure that duplicate notarized deposits do not result in double-minting LBTC. Similarly, for unstaking operations, the off-chain component transferring BTC back to the user should ensure that BTC is transferred once for each unique unstake.

It is assumed that the Starknet LBTC program will maintain its accounts' active status and prevent erasure (e.g., due to insufficient rent fees) until the corresponding request is completed, ensuring process continuity.

Starknet Logs

The system relies on Starknet event logs to confirm that specific actions, such as redeeming assets or initiating protocol operations, have occurred on the Starknet chain. These logs are used to notarize outgoing transactions or validate off-chain processes (e.g., releasing BTC on another chain). However, relying on Starknet logs introduces several trust and security assumptions, as they are not stored on-chain and depend on RPC infrastructure to retrieve them. This can lead to several potential issues:

• Starknet logs are not part of the on-chain state commitment and are not included in the state root posted to Ethereum. As a result, their authenticity cannot be directly verified from L1, and they must be fetched from an RPC endpoint, introducing a reliance on off-chain infrastructure.
• A malicious or compromised RPC operator could forge or manipulate log data returned to the client. This could trick off-chain components into believing that a valid redemption or unstaking operation has occurred, leading to unauthorized asset release.
• The Starknet OS, Cairo VM, or the logging behavior may change in the future, potentially altering how events are emitted or formatted. Such changes could break event parsers and cause system downtime or incorrect interpretation of protocol state.
• The emitting contract could be upgraded to emit events that are identical to those expected from a legitimate redemption, without actually performing the required logic (e.g., burning tokens). Without deeper verification, this could result in BTC being released without a corresponding Starknet action.

Medium Severity

Incompatible scriptPubKey Length for P2WPKH in Starknet Event Decoder

The DecodeUnstakeOnchainMsgFromStarknetEvent function reconstructs scriptPubKey by combining a fixed 31-byte segment with up to 30 additional bytes from a secondary field, resulting in a total length between 31 and 61 bytes. However, a valid P2WPKH (Pay-to-Witness-Public-Key-Hash) script is exactly 22 bytes long:

• 1 byte for the witness version (OP_0).
• 1 byte for the push opcode (0x14 for 20 bytes).
• 20 bytes for the hash of the public key.

As a result, the reconstructed script cannot represent a valid P2WPKH script, despite the verify function listing it as a supported type.

Due to the enforced minimum length of 31 bytes in the decoder, shorter script types such as P2WPKH cannot be correctly reconstructed or verified. Only longer script types like P2WSH and P2TR, which have a 34-byte script structure, fall within the expected length range and will be properly parsed. This effectively excludes P2WPKH from being supported, which may lead to confusion or misconfiguration if users expect it to be a valid option.

Consider aligning the expected structure and the length of the encoded scriptPubKey with the requirements of the supported script types. This may involve revisiting the event encoding strategy, adjusting the decoder to handle shorter scripts correctly, or refining the set of accepted script types to match what can be faithfully reconstructed from on-chain data.

Update: Resolved in pull request #304. The decoding of the event has been adapted to support script types which are less than 31 bytes.

Potential Integer Cast Overflow on 32-Bit Platforms

In the GetUnstake function, a cast is performed from uint32 to int when comparing the eventIndex against the length of the Events slice. Casting a uint32 to int introduces a risk of integer overflow on 32-bit architectures, where the int type is only 32 bits. If eventIndex exceeds math.MaxInt32, the cast can wrap the value into a negative number, resulting in incorrect behavior or panics when accessing the slice.

While this logic is not executed on-chain and does not affect consensus-critical paths, the cast still introduces subtle architecture-dependent behavior that can manifest in crashes or incorrect logic in 32-bit environments, testing setups, or tools running outside of validator processes. This can reduce robustness, increase maintenance complexity, and pose a risk if assumptions about the platform's word size change in the future.

Consider not relying on architecture-dependent types or conversions when performing critical index or bounds checks. Instead, validate that the uint32 value is within a safe range before casting or use explicitly sized types that do not vary across platforms. Ensuring that conversions are bounded and platform-independent will help improve the portability and safety of the codebase.

Update: Resolved in pull request #305. The Lombard team stated:

Fixed by casting the receipts length to uint32 rather than the field from the payload.

Low Severity

Inconsistent Handling of u128 ABI Field in Event Decoder

The DecodeUnstakeOnchainMsgFromStarknetEvent function decodes the amount_after_fee field from the UnstakeRequest event as a uint64 value, whereas the event's ABI specifies the field as a u128 value. This introduces a type mismatch between the on-chain event definition and the off-chain decoder, potentially leading to incorrect behavior if the emitted data exceeds the limits of the chosen type.

This specific field represents BTC amounts, which currently fall well within the range of a uint64. However, relying on this assumption without enforcing it in code may lead to issues if the contract emits values beyond this range, whether due to a bug, misuse, or future protocol changes. In such cases, the decoder could silently truncate values or behave unpredictably, affecting the correctness and reliability of downstream logic.

Consider aligning the off-chain decoder's handling of the amount_after_fee field more closely with the ABI-defined type to ensure compatibility and correctness. This may involve using a type that is capable of representing the full u128 range, or explicitly validating that emitted values are within acceptable bounds before conversion.

Update: Resolved in pull request #306.

Potential Runtime Panic in DecodeUnstakeOnchainMsgFromStarknetEvent

As part of the event decoding logic, the current implementation of the DecodeUnstakeOnchainMsgFromStarknetEvent function may cause runtime panic due to unchecked bounds when slicing into the pendingBytes array.

The function reconstructs the scriptPubKey by appending the last pendingBytesLength bytes from the pendingBytes field. However, pendingBytesLength is derived from event data that is ultimately emitted by a Cairo contract and should not be assumed to be trusted or internally consistent. If pendingBytesLength exceeds the actual length of pendingBytes, this line will trigger a slice bounds out-of-range panic, potentially crashing the notaryd service.

To prevent unexpected crashes, consider validating that pendingBytesLength does not exceed the length of pendingBytes before attempting to slice. This ensures robust handling of malformed or adversarial inputs and guards against decoder instability.

Update: Resolved in pull request #307.

pendingBytesLength is now checked against the length of the pendingBytes field to ensure that it is not resulting in a slice bounds out-of-range panic.

Potential Truncation Risk When Casting pendingBytesLength to int in Event Decoder

In the DecodeUnstakeOnchainMsgFromStarknetEvent function, the code casts a usize-sized value (pendingBytesLength, derived from event.Data[4].Uint64()) to an int when computing the slice offset for pendingBytes. On most modern systems, this cast will work as expected. However, on 32-bit architectures, or in constrained environments where int is limited to 32 bits, a large pendingBytesLength could exceed math.MaxInt32 and wrap into a negative value. This could result in an out-of-bounds slice offset, leading to panics or incorrect data reconstruction.

While pendingBytesLength is expected to be at most 30 (1 byte) under normal conditions (to complete the reconstruction of a scriptPubKey from the initial 31-byte segment), it is ultimately derived from untrusted on-chain data. If the contract emits an unexpectedly large pendingBytesLength, the cast to int could overflow on 32-bit architectures. This may result in a negative slice offset and trigger a slice bounds out-of-range panic, potentially crashing the notaryd service. Even if such inputs are unlikely under typical execution, failing to validate this boundary exposes the decoder to unexpected crashes in edge cases.

Consider validating that pendingBytesLength fits safely within the bounds of the target integer type and does not exceed the length of the pendingBytes array before casting or slicing. This ensures the resilient handling of untrusted event data and protects the decoder against architecture-specific truncation errors and potential runtime panics.

Update: Resolved in pull request #307. The Lombard team stated:

Now, the pending byte field has a maximum size of 32 bytes.

Unhandled Errors in notaryd Configuration and Shutdown Processes

The notarization service exhibits unhandled errors in two areas of its operation. These areas are the configuration override mechanism and the shutdown procedure, which are essential for the reliable and efficient functioning of the notaryd process. Although these might not have major effects on the system, they could lead to process termination with unclear error messages. This lack of clarity in error reporting can complicate debugging efforts and hinder the rapid resolution of issues.

The configuration override process involves overriding default configurations with those specified via Viper. This step is crucial for customizing the process' behavior according to user or environmental requirements. However, errors during this override process are not currently handled, which could lead to the process starting with incorrect or unintended configurations. Such misconfigurations could adversely affect the operation of notarization service, potentially leading to suboptimal performance or unexpected behavior.

The shutdown procedure is involved in the shutdown process of the notarization service. While it appears to perform proper cleanup operations, errors that may occur during this phase are not handled.

Consider implementing comprehensive error handling in both the configuration override and shutdown procedures. Doing so would not only facilitate clearer debugging but also enhance the overall robustness and reliability of the notaryd component.

Update: Resolved in pull request #308 and pull request #316.

Notes & Additional Information

Missing Logging for Configured Starknet Chains in runNotaryd

The runNotaryd function logs the configuration details for the Bitcoin, EVM, Sui, and Solana chains at startup, providing useful visibility into which networks are being monitored and how they are configured. However, there is no equivalent log output for configured Starknet chains.

This omission reduces observability for Starknet integration and can make it harder for operators and developers to verify that the correct Starknet chains are configured and active. It also breaks consistency with the logging approach used for other supported chains, which may lead to confusion or missed misconfigurations during deployment and runtime debugging.

Consider adding a log section for the configured Starknet chains in runNotaryd, following the same pattern used for other chains. This would enhance transparency, aid operational monitoring, and bring consistency to the startup logging behavior across all supported blockchain networks.

Update: Resolved in pull request #309.

Misleading Inline Documentation

The inline documentation for the NewStarknetSepoliaLChainId function incorrectly states that it returns the Lombard Chain ID for Starknet mainnet (SN_MAIN). However, the function implementation actually returns the Lombard Chain ID for Starknet Sepolia (SN_SEPOLIA), which creates a mismatch between the documented behavior and the actual output.

Consider updating the inline documentation to accurately reflect that the function returns the Lombard Chain ID for Starknet Sepolia (SN_SEPOLIA). Clear and correct documentation ensures that developers can rely on it when working with environment-specific logic and reduces the likelihood of subtle bugs.

Disclaimer: The lombard-finance/ledger-utils repository is technically out of scope, but the issue above is noted here for completeness and potential correction.

Update: Resolved in pull request #8.

Uninitialized StarknetConfigs Map in DefaultNotaryConfig

The DefaultNotaryConfig function, integral to the configuration setup of the notarization service (notaryd), is designed to establish default settings for various blockchain configurations. However, in the current implementation, the StarknetConfigs map, which is expected to hold configuration details specific to the Starknet chain, is not initialized and, thus, defaults to a nil map.

This uninitialized map poses a risk of runtime panics when operations attempt to access or modify the StarknetConfigs map. Such panics can lead to abrupt termination of the notaryd service, affecting its stability and reliability. Moreover, the absence of default configurations for Starknet within the DefaultNotaryConfig function may hinder the seamless integration and operation of Starknet within the Lombard Finance ecosystem.

Consider initializing the StarknetConfigs map in the DefaultNotaryConfig function. This would mitigate these risks and ensure a consistent and robust configuration framework.

Update: Resolved in pull request #310. 

Conclusion

This review examined the recent changes made to the Lombard Ledger codebase, focusing on the newly added support for Starknet as a target chain for BTC staking, LBTC minting, and unstaking features within the notaryd auxiliary service. Two issues of medium severity and several opportunities for code improvement were identified. Overall, the code under review was found to be clean, well-structured, and to demonstrate a deliberate attempt at ensuring compatibility with Starknet.