Prepared by:
HALBORN
Last Updated 02/11/2025
Date of Engagement: December 23rd, 2024 - January 7th, 2025
100% of all REPORTED Findings have been addressed
All findings
9
Critical
0
High
0
Medium
0
Low
3
Informational
6
BitHive
engaged Halborn to conduct a security assessment of their Bitcoin staking protocol for NEAR blockchain, beginning on December 23rd, 2024, and ending on January 7th, 2025. The security assessment was scoped to the repository listed with commit hash, and further details in the Scope section of this report.
The BitHive
Bitcoin staking protocol allows bitcoin holders to stake their Bitcoin, without needing any third-party custody/bridge/wrapping.
The team at Halborn assigned one full-time security engineer to verify the security of the smart contracts. The security engineer is a blockchain and smart-contract security expert with advanced penetration testing and smart-contract hacking skills, and deep knowledge of multiple blockchain protocols.
The purpose of this assessment is to:
Ensure that smart contract functions operate as intended.
Identify potential security issues with the smart contracts.
In summary, Halborn identified some improvements to reduce the likelihood and impact of risks, which were mostly addressed by the BitHive team
. The main ones were the following:
Refund the actual deposited amount to the caller, rather than just the minimum attached amount.
Create low-privilege roles for executing routine tasks within the protocol to minimize the impact of potential compromises of these accounts.
Implement a two-step ownership transfer process.
Halborn performed a combination of the manual view of the code and automated security testing to balance efficiency, timeliness, practicality, and accuracy regarding the scope of the smart contract assessment. While manual testing is recommended to uncover flaws in logic, process, and implementation, automated testing techniques help enhance the coverage of smart contracts. They can quickly identify items that do not follow security best practices. The following phases and associated tools were used throughout the term of the assessment:
Research into the architecture, purpose, and use of the platform.
Manual code reading and walkthrough.
Manual assessment of the use and safety of critical Rust variables and functions to identify potential arithmetic vulnerabilities.
Cross-contract call controls.
Logical controls related to architecture.
Scanning of Rust files for vulnerabilities using tools like cargo audit
.
Integration testing using the NEAR testing framework.
EXPLOITABILITY METRIC () | METRIC VALUE | NUMERICAL VALUE |
---|---|---|
Attack Origin (AO) | Arbitrary (AO:A) Specific (AO:S) | 1 0.2 |
Attack Cost (AC) | Low (AC:L) Medium (AC:M) High (AC:H) | 1 0.67 0.33 |
Attack Complexity (AX) | Low (AX:L) Medium (AX:M) High (AX:H) | 1 0.67 0.33 |
IMPACT METRIC () | METRIC VALUE | NUMERICAL VALUE |
---|---|---|
Confidentiality (C) | None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C) | 0 0.25 0.5 0.75 1 |
Integrity (I) | None (I:N) Low (I:L) Medium (I:M) High (I:H) Critical (I:C) | 0 0.25 0.5 0.75 1 |
Availability (A) | None (A:N) Low (A:L) Medium (A:M) High (A:H) Critical (A:C) | 0 0.25 0.5 0.75 1 |
Deposit (D) | None (D:N) Low (D:L) Medium (D:M) High (D:H) Critical (D:C) | 0 0.25 0.5 0.75 1 |
Yield (Y) | None (Y:N) Low (Y:L) Medium (Y:M) High (Y:H) Critical (Y:C) | 0 0.25 0.5 0.75 1 |
SEVERITY COEFFICIENT () | COEFFICIENT VALUE | NUMERICAL VALUE |
---|---|---|
Reversibility () | None (R:N) Partial (R:P) Full (R:F) | 1 0.5 0.25 |
Scope () | Changed (S:C) Unchanged (S:U) | 1.25 1 |
Severity | Score Value Range |
---|---|
Critical | 9 - 10 |
High | 7 - 8.9 |
Medium | 4.5 - 6.9 |
Low | 2 - 4.4 |
Informational | 0 - 1.9 |
Critical
0
High
0
Medium
0
Low
3
Informational
6
Security analysis | Risk level | Remediation Date |
---|---|---|
Flawed Refund Mechanism During Deposits | Low | Solved - 01/14/2025 |
Centralization Risk | Low | Risk Accepted - 12/30/2024 |
Single-Step Ownership Transfer Process | Low | Solved - 01/15/2025 |
Lack Of Prepaid Gas Validation May Result In Unintended Consumption Of Gas | Informational | Solved - 01/14/2025 |
Inadequate Error Message | Informational | Solved - 01/14/2025 |
Unchecked Math Operations | Informational | Acknowledged - 01/23/2025 |
Presence Of Typographical Error | Informational | Solved - 01/14/2025 |
Misleading Comment | Informational | Solved - 01/14/2025 |
Missing Event Emission | Informational | Partially Solved - 01/14/2025 |
//
To make a deposit, the user sends BTC to a P2WSH
(Pay-to-Witness-Script-Hash) address, where the UTXO (Unspent Transaction Output) can only be spent if either:
It's signed by both the BitHive contract's MPC signature and the user's BTC private key, or
After a lock-up period, it's signed solely by the user's BTC private key.
Then, off-chain relayers identify the deposit transaction and submit it to the bithive contract by calling the submit_deposit_tx
function, which verifies its validity using the BTC light client and records the user's deposit details. This function is payable and requires the caller to attach a minimum of STORAGE_DEPOSIT_ACCOUNT
(0.03 NEAR) to cover the storage fee.
However, if the provided deposit transaction is deemed invalid, the callback refunds only the STORAGE_DEPOSIT_ACCOUNT
amount (0.03 NEAR) instead of the actual attached deposit, which could be larger.
As a result, the relayer may incur a loss of funds.
Below is a code snippet of the submit_deposit_tx
function:
require!(
env::attached_deposit() >= STORAGE_DEPOSIT_ACCOUNT,
ERR_NOT_ENOUGH_STORAGE_DEPOSIT
);
Below is a code snippet of the on_verify_deposit_tx
function:
if !valid {
self.unset_deposit_confirmed(&txid.to_string().into(), deposit_vout);
// refund storage deposit
Promise::new(caller_id).transfer(STORAGE_DEPOSIT_ACCOUNT);
return false;
}
It is recommended to refund the actual deposited amount to the caller, rather than just the minimum attached amount (STORAGE_DEPOSIT_ACCOUNT
).
SOLVED: The BitHive team solved the issue in the specified commit id.
//
The bithive contract designates an owner during installation, granting full control over the upgrade process and the ability to configure critical protocol parameters in bithive/src/admin.rs
.
This violates the principle of separation of duties, introduces centralization risks, and grants the owner absolute control over the entire contract.
It is recommended to create low-privilege roles for executing routine tasks within the protocol to minimize the impact of potential compromises of these accounts.
RISK ACCEPTED: The BitHive team accepted the risk of this finding and stated the following:
Currently we have only one owner role that can change configurations of the smart contract. There's another relayer operator role which can call functions like submit_deposit_tx
, queue_withdrawal
, sign_withdrawal
and submit_withdrawal_tx
, but all these functions are public and are callable by anyone, without permission control. So I think we don't need to introduce low-privilege roles.
//
The ownership transfer of the bithive contract is executed in a single step, which introduces a security risk. This issue is present in the admin::change_owner
function, where the change of ownership lacks a verification step before finalization. As a result, there is a risk that the owner could make a mistake when executing the ownership transfer, such as setting an incorrect address, which could lead to the contract being locked or inaccessible.
Below is the implementation of the change_owner
function:
#[payable]
pub fn change_owner(&mut self, new_owner_id: AccountId) {
self.assert_owner();
self.owner_id = new_owner_id;
}
It is recommended to implement a two-step ownership transfer process: one function to establish the new owner's address (propose_new_owner
) and another to accept the transfer (accept_ownership
). The latter function should only be executable by the new owner.
SOLVED: The BitHive team solved the issue in the following commit IDs:
8157df2: This commit includes the following changes:
Removed the change_owner
function.
Added the pending_owner_id
variable to the bithive
contract struct.
Introduced the new propose_change_owner
function, which is called by the current owner to set a new owner account as pending_owner_id
.
Added the accept_change_owner
function, which is called by the pending owner to finalize the ownership transfer by setting the current owner as the pending_owner_id
.
92c33aa: In this commit, 2FA was implemented in the accept_change_owner
function using assert_one_yocto()
, as it is a privileged function.
//
To unstake BTC, users must first initiate a queue withdrawal request by signing BTC messages. This request is then processed by a relayer on the NEAR side, which invokes the queue_withdrawal
function within the bithive contract.
However, the queue_withdrawal
function currently lacks a verification step to ensure that the caller has attached sufficient execution gas. Consequently, if the attached gas is insufficient, the function will fail and consume all of the attached gas. This approach prevents an early revert, which could have preserved some of the gas for the user.
It is recommended to verify that the prepaid gas is sufficient to cover the execution gas requirements at the beginning of the queue_withdrawal
function. This precaution will help prevent unnecessary execution attempts that could exhaust all attached gas, ensuring a more efficient use of resources.
SOLVED: The BitHive team solved the issue in the specified commit id.
//
Within the dry_run_deposit
function, the provided deposit transaction is verified to ensure it has not already been confirmed before proceeding with the transaction verification process. If the deposit transaction is found to be already confirmed, the transaction reverts with the following message: "deposit txn verification failed". However, this message does not accurately convey the specific issue, as the transaction's failure is due to its prior confirmation, not an inherent problem with the transaction verification itself.
Below is the implementation of the dry_run_deposit
function:
pub fn dry_run_deposit(&self, tx_hex: String, embed_vout: u64) {
self.assert_running();
let tx = deserialize_hex::<Transaction>(&tx_hex).unwrap();
let output_id = output_id(&tx.compute_txid().to_string().into(), embed_vout);
require!(
!self.confirmed_deposit_txns.contains(&output_id),
"deposit txn verification failed"
);
self.verify_deposit_txn(&tx, embed_vout);
}
It is recommended to update the error message to something more descriptive and accurate, such as: "Deposit transaction already confirmed". This would clearly convey the reason for the failure, helping users better understand the issue.
SOLVED: The BitHive team solved the issue in the specified commit id.
//
In computer programming, an overflow occurs when an arithmetic operation attempts to create a numeric value that is outside the range that can be represented with a given number of bits – either larger than the maximum or lower than the minimum representable value. This is a good practice, since having overflow-checks
enabled in the workspace would avoid overflow situations in release
mode.
While the likelihood of an overflow issue arising from the identified occurrences is low, it's still best practice to handle overflow errors gracefully.
Below is a list of identified math operations that remain unchecked:
contracts/bithive/src/account.rs
L140: self.total_deposit += value;
L155: self.queue_withdrawal_amount + amount <= self.total_deposit
L158: self.queue_withdrawal_amount += amount;
L160: self.nonce += 1;
L177: self.total_deposit -= deposit.value;
contracts/bithive/src/withdraw.rs
L324: account.queue_withdrawal_start_ts + self.withdrawal_waiting_time_ms
L351: let actual_withdraw_amount = deposit_input_sum - reinvest_amount;
contracts/bithive/src/utils/btc.rs
L55: let flag = sig[0] - 27;
It is recommended to utilize checked arithmetic operations to handle overflow errors gracefully.
ACKNOWLEDGED: The BitHive team acknowledged this issue and stated the following:
Since we already have overflow-checks = true
in release build, all arithmetic operations will be protected and the contract will panic if overflow happened. The reason why we think using checked math is unnecessary is that even if it is used, what we can do in case of overflow is still abort the execution and panic, there is no way we can recover from this kind of error. So basically this would have the same effect as it is now.
//
The following typographical error was identified:
contracts/bithive/src/deposit.rs
L23: const ERR_BAD_PUBKER_HEX: &str = "Invalid pubkey hex";
It is recommended to fix the aforementioned typographical error as follows:
ERR_BAD_PUBKER_HEX
-> ERR_BAD_PUBKEY_HEX
SOLVED: The BitHive team solved the issue in the specified commit id.
//
Incorporating documentation and comments into the codebase is essential for elucidating key aspects and facilitating comprehension of the developer's intentions by others. However, the codebase contains a misleading comment in the verify_pending_sign_request_amount
function, where the actual withdrawal amount is incorrectly documented as being 'less than' the requested withdrawal amount, rather than 'less than or equal to', as shown below:
// make sure the actual amount is less than the requested withdrawal amount
require!(
actual_withdraw_amount <= account.queue_withdrawal_amount,
ERR_BAD_WITHDRAWAL_AMOUNT
);
It is recommended to correct the misleading comment for accuracy.
SOLVED: The BitHive team solved the issue in the specified commit id.
//
The bithive contract implements owner-only functions that modify critical state variables without emitting events. The affected functions are the following:
change_owner
set_btc_light_client_id
set_bip322_verifier_id
set_chain_signatures_id
set_n_confirmation
set_withdrawal_waiting_time
set_min_deposit_satoshi
set_earliest_deposit_block_height
set_solo_withdrawal_sequence_heights
set_paused
The absence of events prevents external systems or users from tracking critical administrative changes via event logs. This lack of transparency diminishes visibility into owner actions that impact the protocol's behavior.
It is recommended to emit an event whenever important contract state variables are updated.
PARTIALLY SOLVED: The BitHive team partially solved this issue. They chose to emit events only in the accept_change_owner
and set_paused
functions and stated the following:
We choose not to emit events for all these operations because based on our business logic and actual usage, in most cases users won't need to track the updates of these parameters, they just need to query the latest value before interacting with us.
Halborn used automated security scanners to assist with detection of well-known security issues and vulnerabilities. Among the tools used was cargo audit
, a security scanner for vulnerabilities reported to the RustSec Advisory Database. All vulnerabilities published in https://crates.io
are stored in a repository named The RustSec Advisory Database. cargo audit
is a human-readable version of the advisory database which performs a scanning on Cargo.lock. Security Detections are only in scope. To better assist the developers maintaining this code, the auditors are including the output with the dependencies tree, and this is included in the cargo audit
output to better know the dependencies affected by unmaintained and vulnerable crates.
ID | package | Short Description |
---|---|---|
curve25519-dalek 3.2.0 | Timing variability in | |
ed25519-dalek 1.0.1 | Double Public Key Signing Function Oracle Attack on | |
wee_alloc 0.4.5 | wee_alloc is Unmaintained | |
borsh 0.9.3 | Parsing borsh messages with ZST which are not-copy/clone is unsound |
Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed