Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@

## Audit Status

**NOT AUDITED** - Use at your own risk.
**AUDITED** - AuditAgent scan completed January 4, 2026 (Scan ID: 26).

See [audits/](audits/) directory for full reports.

## Security Contact

Report vulnerabilities to: security@liq.protocol
Report vulnerabilities via [GitHub Issues](https://github.com/igor53627/liq/issues) with the `security` label.

For sensitive disclosures, use [GitHub's private vulnerability reporting](https://github.com/igor53627/liq/security/advisories/new).

## Known Security Considerations

Expand Down Expand Up @@ -35,15 +39,16 @@ Report vulnerabilities to: security@liq.protocol

**Design Decision**: `poolBalance` is tracked in storage instead of calling `balanceOf()` before each loan.

**Invariant**: `poolBalance` must always equal actual USDC balance.
**Invariant**: `poolBalance` should equal actual USDC balance.

**Maintained by**:
- `deposit()`: poolBalance += amount
- `withdraw()`: poolBalance -= amount
- Flash loan: verified via `balanceOf() >= poolBalance` after callback
- Reentrancy guard: prevents deposit/withdraw during callback
- `sync()`: owner can call to set poolBalance = actual balance

**Risk**: If someone sends USDC directly to contract (not via deposit), poolBalance will be less than actual balance. This is safe - it just means extra USDC is locked until owner withdraws.
**Design Note - Direct Transfers**: The owner deposits USDC via `deposit()`, not by sending USDC directly to the contract. This design saves gas by avoiding an extra balance check. If someone accidentally sends USDC directly, the owner can call `sync()` to claim it. Any excess USDC (from direct transfers) may be extracted via flash loan before `sync()` is called - this is accepted behavior since direct transfers are not the intended deposit method.

### 4. Reentrancy Guard

Expand Down
Binary file not shown.
87 changes: 87 additions & 0 deletions audits/audit_responses.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Audit Responses

This document tracks responses to audit findings that are either false positives, acknowledged design decisions, or out of scope.

## AuditAgent Report - January 4, 2026 (Scan ID: 26)

### False Positives / Scanner Misunderstandings

**Finding #4: Unsafe Return Statement In Yul Block** - FALSE POSITIVE
- The `return(0x00, 0x20)` statements in Yul are intentional and correct
- They are used in deliberate branches with no intended fallthrough
- This is standard Yul pattern for returning values from function selectors

**Finding #12: Unused State Variable** - FALSE POSITIVE
- The `locked` and `USDC` variables are declared for storage layout/ABI purposes
- They are accessed via Yul assembly using slot numbers, not Solidity variable names
- The scanner doesn't recognize Yul-based access patterns

### Acknowledged Design Decisions

**Finding #5: Non-Specific Solidity Pragma Version** - ACKNOWLEDGED
- Using `^0.8.20` is acceptable as Foundry pins the compiler version
- Risk is mitigated by CI testing with specific compiler version

**Finding #6: PUSH0 Opcode Compatibility Issue** - ACKNOWLEDGED
- Contract is deployed only on Ethereum mainnet which supports Shanghai
- L2 deployment is not planned; if needed, would require recompilation

**Finding #9: ERC-3156 non-compliance (callback return value)** - ACKNOWLEDGED
- Intentional gas optimization, documented in SECURITY.md
- Balance check provides equivalent security guarantee
- Documentation updated to clarify "ERC-3156 compatible (except callback return value verification)"

**Finding #10: Permissionless deposit() has no depositor accounting** - ACKNOWLEDGED
- This is the intended design - deposits are effectively donations to the pool
- Owner controls all withdrawals
- Documented in README and SECURITY.md

**Finding #11: High Function Complexity** - ACKNOWLEDGED
- The fallback() function handles multiple selectors in Yul for gas optimization
- Complexity is inherent to the selector-dispatch pattern
- Well-documented and tested

**Finding #13/#14: Missing events for critical operations** - ACKNOWLEDGED (Issue #21 closed)
- Events were intentionally omitted to save gas in the Yul implementation
- This is a core design choice for the gas-optimized flash loan protocol
- Flash loans DO emit a `FlashLoan` event; only admin functions omit events
- Users can track deposits/withdrawals via USDC Transfer events

**Finding #15: No mechanism to rescue non-USDC tokens** - ACKNOWLEDGED
- Contract is USDC-only by design
- Adding rescue function would increase attack surface
- Users should not send non-USDC tokens to the contract

**Finding #8: Missing ERC20 return value checks** - FUTURE VERSION (Issue #20 closed)
- Valid concern for a future redeployment
- Current contract relies on USDC reverting on transfer failure (current behavior)
- Since contract is stateless (poolBalance can be re-synced via `sync()`), redeployment is low-friction
- For next version: add `if iszero(mload(0x00)) { revert(0, 0) }` after ERC20 calls

### Out of Scope (Example/Test Code)

**Finding #1: TestBorrower arbitrary lender injection** - TRACKED AS ISSUE #19
- Real vulnerability but in example/test contract, not production code
- Created issue to harden the example for safety of integrators who may copy it

**Finding #7: Unsafe ERC20 Operation Usage (TestBorrower)** - OUT OF SCOPE
- TestBorrower is example code, not production
- Would be fixed as part of Issue #19

### Acknowledged Design Decisions (Additional)

**Finding #2/#3: Excess USDC extraction via flash loan** - ACKNOWLEDGED (Issue #18 closed)
- The owner deposits USDC via `deposit()`, not by sending USDC directly to the contract
- This design saves gas by avoiding an extra balance check
- In normal operation, there should never be excess USDC (actualBalance > poolBalance)
- If someone accidentally sends USDC directly, the owner can call `sync()` to claim it
- Any excess USDC may be extracted via flash loan before `sync()` is called - this is accepted behavior

### Summary - All Issues Resolved

| Finding | Severity | Issue | Status |
|---------|----------|-------|--------|
| #2/#3: Excess USDC extraction | Medium | [#18](https://github.com/igor53627/liq/issues/18) | Closed - Design decision |
| #1: TestBorrower lender injection | High (example code) | [#19](https://github.com/igor53627/liq/issues/19) | Fixed in PR #22 |
| #8: Missing ERC20 return value checks | Info | [#20](https://github.com/igor53627/liq/issues/20) | Closed - Future version |
| #13/#14: Missing events | Best Practices | [#21](https://github.com/igor53627/liq/issues/21) | Closed - Gas optimization |
15 changes: 14 additions & 1 deletion src/TestBorrower.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,45 @@ interface IFlashLender {

/// @title TestBorrower
/// @notice Simple flash loan borrower for testing - borrows and repays immediately
/// @dev Fixed to prevent arbitrary lender injection attacks
contract TestBorrower {
bytes32 constant CALLBACK_SUCCESS = keccak256("ERC3156FlashBorrower.onFlashLoan");
address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;

/// @notice Tracks the expected lender during a flash loan to prevent unauthorized callbacks
address private expectedLender;
/// @notice Tracks the expected amount to prevent amount manipulation in callback
uint256 private expectedAmount;

event FlashLoanExecuted(address lender, uint256 amount);

/// @notice Borrow with event (for testing/debugging)
function borrow(address lender, uint256 amount) external {
expectedLender = lender;
expectedAmount = amount;
IFlashLender(lender).flashLoan(address(this), USDC, amount, "");
expectedLender = address(0);
expectedAmount = 0;
emit FlashLoanExecuted(lender, amount);
}

/// @notice Borrow without event (gas optimized for production)
function borrowSilent(address lender, uint256 amount) external {
expectedLender = lender;
expectedAmount = amount;
IFlashLender(lender).flashLoan(address(this), USDC, amount, "");
expectedLender = address(0);
expectedAmount = 0;
}

function onFlashLoan(address, address, uint256 amount, uint256, bytes calldata) external returns (bytes32) {
function onFlashLoan(address initiator, address token, uint256 amount, uint256, bytes calldata)
external
returns (bytes32)
{
require(msg.sender == expectedLender, "unauthorized callback");
require(initiator == address(this), "invalid initiator");
require(token == USDC, "invalid token");
require(amount == expectedAmount, "amount mismatch");
bool success = IERC20(USDC).transfer(msg.sender, amount);
require(success, "transfer failed");
return CALLBACK_SUCCESS;
Expand Down