Skip to content

[VPD-1598] Hashdit Audit Mitigations for BstockLiquidator#696

Open
Debugger022 wants to merge 8 commits into
feat/VPD-1430from
feat/VPD-1598
Open

[VPD-1598] Hashdit Audit Mitigations for BstockLiquidator#696
Debugger022 wants to merge 8 commits into
feat/VPD-1430from
feat/VPD-1598

Conversation

@Debugger022

Copy link
Copy Markdown
Contributor

Summary

  • Remediates the HashDit security audit of BStockLiquidator: five Low findings (L02–L06) and one Informational (I01) are fixed; the Medium (M01) and one Low (L01) are acknowledged as latent/out-of-threat-model.
  • No fund-loss issues were found. Changes harden observability, prevent an owner footgun, correct an off-chain estimation overload, and tighten the deploy checklist and quote-freshness handling.

Changes

Contract (contracts/BStock/)

  • L02_swap now measures the input actually consumed (balBefore - balAfter) and emits PartialSwapLeftover when a router fills less than approved (e.g. a partial RFQ fill), leaving the residual observable and sweepable; overstated NatSpec corrected.
  • L03renounceOwnership() overridden to a no-op, so an accidental call cannot zero the owner and brick this fund-custody contract.
  • I01sweepNative given nonReentrant for consistency with the liquidation entrypoints (defense-in-depth).

Off-chain scripts (scripts/bstock/)

  • L04atomic-liquidate.ts and safe-fallback.ts switched to the borrower-aware 4-arg liquidateCalculateSeizeTokens overload (matches vToken.liquidateBorrowFresh); added seizeTokens > 0 sanity guards.
  • L05 — deploy checklist documents the governance-gated setWhiteListFlashLoanAccount step required before FLASH mode.
  • L06atomic-liquidate.ts re-verifies the Native quote's remaining TTL immediately before submission, aborting below a configurable SETTLE_TTL_MARGIN (default 10s).

Tests & mocks

  • Fork helper + call sites updated to the 4-arg overload (L04); MockComptrollerLite gained the 4-arg overload.
  • New unit tests: partial-fill event + sweep recovery and no-emit on full fill (L02), renounceOwnership no-op (L03), TTL-below-margin abort + invalid-margin rejection (L06).

Test plan

  • npx hardhat compile — clean.
  • npx hardhat test tests/hardhat/BStockLiquidator.ts tests/hardhat/BStockAtomicLiquidate.ts tests/hardhat/BStockSafeFallback.ts — 64 passing.
  • Fork suite (tests/hardhat/Fork/BStockLiquidatorFork.ts) requires a BSC archive RPC.

Audit disposition

Finding Severity Status
M01 Medium Acknowledged — forceVAILiquidate is false on mainnet, gate condition currently unreachable; governance-gated with an off-chain workaround
L01 Low Acknowledged — operators are a trusted role by design; FLASH mode already enforces an operator-independent floor
L02–L06 Low Fixed
I01 Informational Fixed

@Debugger022 Debugger022 self-assigned this Jul 13, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements the HashDit audit mitigations for BStockLiquidator, spanning on-chain hardening (swap accounting + ownership footgun prevention), off-chain liquidation tooling correctness (borrower-aware seize estimation + quote freshness checks), and corresponding tests/mocks to validate the new behaviors.

Changes:

  • Adds PartialSwapLeftover observability for partial-fill swap hops and updates swap accounting to measure actual input consumption.
  • Disables renounceOwnership() to prevent bricking owner-only recovery/admin controls; adds nonReentrant to sweepNative for defense-in-depth consistency.
  • Updates scripts and fork helpers to use the borrower-aware 4-arg liquidateCalculateSeizeTokens overload and adds a pre-submit Native-quote TTL margin check (with tests).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/hardhat/Fork/helpers/bstock.ts Fork helper updated to use borrower-aware 4-arg seize calculation and thread borrower through two-hop builder.
tests/hardhat/Fork/BStockLiquidatorFork.ts Updates fork test call sites to pass borrower into the two-hop helper.
tests/hardhat/BStockLiquidator.ts Adds unit tests for partial-fill leftover event + sweep recovery and renounceOwnership no-op semantics.
tests/hardhat/BStockAtomicLiquidate.ts Adds tests for SETTLE_TTL_MARGIN validation and TTL-below-margin abort behavior.
scripts/bstock/safe-fallback.ts Switches to 4-arg seize overload and adds seizeTokens > 0 sanity guard.
scripts/bstock/atomic-liquidate.ts Switches to 4-arg seize overload, adds seizeTokens > 0 guard, and re-checks quote TTL against SETTLE_TTL_MARGIN immediately before submit.
deploy/019-deploy-bstock-liquidator.ts Documents governance-gated flash-loan whitelist step required for FLASH mode.
contracts/test/BStockLiquidationMocks.sol Extends MockComptrollerLite with the 4-arg liquidateCalculateSeizeTokens overload.
contracts/BStock/IBStockLiquidator.sol Introduces PartialSwapLeftover event to surface unconsumed input residuals.
contracts/BStock/BStockLiquidator.sol Implements leftover emission + swap accounting, adds nonReentrant to sweepNative, and disables renounceOwnership.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread contracts/BStock/BStockLiquidator.sol Outdated
Comment thread contracts/BStock/BStockLiquidator.sol
Comment thread contracts/BStock/BStockLiquidator.sol Outdated
Surface the swap router's own revert reason instead of an opaque
SwapFailed(), so a failed hop reports the actual cause (e.g. insufficient
allowance). SwapFailed() is kept only as the empty-returndata fallback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants