Open sherlock-admin3 opened 2 months ago
Need further investigation.
The graph here (mentioned in the report) should show exactly what was described in the issue. The graph uses the correct formulas as in the codebase. The report explains the graph.
POC to show spread does not change when swap is divided into small swaps
create a 3_woofi_spread.ts
file in tests/
dir alongside 0_woofi.ts
, ... and copy the following contents to the file:
import * as anchor from "@coral-xyz/anchor";
import * as borsh from "borsh";
import { BN, Program } from "@coral-xyz/anchor";
import * as token from "@solana/spl-token";
import { LAMPORTS_PER_SOL, SystemProgram, Transaction, sendAndConfirmTransaction } from "@solana/web3.js";
import { getLogs } from "@solana-developers/helpers";
import { assert } from "chai";
import { createAssociatedTokenAccount, transferToken } from "./utils/token";
import { runQuery } from "./utils/pyth";
import { PoolUtils } from "./utils/pool";
import { getCluster } from "./global";
import { usdcTokenMint, solTokenMint, solPriceUpdate, usdcPriceUpdate, confirmOptionsRetryTres, SupportedToken } from "./utils/test-consts";
import moment from "moment";
describe("woofi_spread", () => {
const poolUtils = new PoolUtils();
poolUtils.initEnv();
const provider = poolUtils.provider;
const program = poolUtils.program;
// SOL/USD
const solFeedAccount = poolUtils.solFeedAccount;
// USDC/USD
const usdcFeedAccount = poolUtils.usdcFeedAccount;
const quoteFeedAccount = usdcFeedAccount;
// Note: test account only in devnet
const keypair = anchor.web3.Keypair.fromSecretKey(
Uint8Array.from([
14, 134, 28, 211, 88, 74, 30, 241, 77, 166, 34,
106, 198, 235, 100, 159, 82, 127, 72, 10, 101, 146,
137, 83, 43, 25, 38, 10, 26, 217, 248, 64, 165,
109, 48, 119, 128, 14, 170, 92, 99, 37, 71, 77,
90, 116, 13, 67, 176, 214, 9, 47, 46, 103, 197,
222, 76, 186, 193, 143, 114, 203, 154, 225
]),
)
const tenpow18 = new BN(10).pow(new BN(18));
const tenpow16 = new BN(10).pow(new BN(16));
const tenpow15 = new BN(10).pow(new BN(15));
const tenpow12 = new BN(10).pow(new BN(12));
const tenpow14 = new BN(10).pow(new BN(14));
// coeff = 0.000003
const testCoeff = new BN(3).mul(tenpow12);
// spread = 0.0005
const testSpread = new BN(5).mul(tenpow14);
// max current feasible price
let testPrice;
// for swap test usage,
// swap payer wallet
const fromWallet = anchor.web3.Keypair.generate();
let signers: anchor.web3.Keypair[] = [fromWallet];
let ataSigner: anchor.web3.Keypair[] = [];
let payerWallet = provider.wallet;
if (getCluster() == 'localnet') {
payerWallet = keypair;
signers.push(keypair);
ataSigner.push(payerWallet);
}
let payerSolTokenAccount: anchor.web3.PublicKey;
let payerUsdcTokenAccount: anchor.web3.PublicKey;
let wooconfigAccount;
let solWooracle;
let solPool;
let usdcPool;
let solTokenVault;
let usdcTokenVault;
let pythoracle_price: BN;
let pythoracle_decimal: number;
let traderSetPrice;
const getPoolData = async (woopool: anchor.web3.PublicKey) => {
let woopoolData = null;
try {
woopoolData = await program.account.wooPool.fetch(woopool);
} catch (e) {
console.log(e);
console.log("Pool fetch failed");
return;
}
return woopoolData;
}
const getWooOracleData = async (wooracle: anchor.web3.PublicKey) => {
let wooracleData = null;
try {
wooracleData = await program.account.wooracle.fetch(wooracle);
} catch (e) {
console.log(e);
console.log("Wooracle fetch failed");
return;
}
return wooracleData;
}
const syncPythPrice = async () => {
const pythPriceFeed = await runQuery();
const solPrice = pythPriceFeed[0].getPriceNoOlderThan(1000);
console.log("solPrice", solPrice);
const usdcPrice = pythPriceFeed[1].getPriceNoOlderThan(1000);
console.log("usdcPrice", usdcPrice);
// use usdc as quote token
pythoracle_decimal = Math.abs(solPrice.expo);
pythoracle_price = new BN(solPrice.price).mul(new BN(10).pow(new BN(pythoracle_decimal))).div(new BN(usdcPrice.price));
const updatedAt = moment.unix(solPrice.publishTime);
console.log(`pythoracle_price:${pythoracle_price}`);
console.log(`pythoracle_decimal:${pythoracle_decimal}`);
console.log(`updated at - ${updatedAt}`);
// TODO Prince: The price is the latest pyth price
// May slightly differ from the one in wooracle's pyth price (clone program in localnet)
traderSetPrice = pythoracle_price;
}
describe("#test_spread_not_updated", async () => {
it("fails to update spread", async () => {
await syncPythPrice();
console.log("1. Mint 150 SOL to from wallet");
payerSolTokenAccount = await createAssociatedTokenAccount(
provider,
solTokenMint,
fromWallet.publicKey,
payerWallet.publicKey,
ataSigner
);
payerUsdcTokenAccount = await createAssociatedTokenAccount(
provider,
usdcTokenMint,
fromWallet.publicKey,
payerWallet.publicKey,
ataSigner
);
const solTokenAccount = payerSolTokenAccount;
const usdcTokenAccount = payerUsdcTokenAccount;
let fromAmount = 150 * LAMPORTS_PER_SOL;
await provider.connection.confirmTransaction(
await provider.connection.requestAirdrop(
fromWallet.publicKey,
fromAmount,
)
);
const fromPoolParams = await poolUtils.generatePoolParams(solTokenMint, usdcTokenMint, solFeedAccount, solPriceUpdate);
const toPoolParams = await poolUtils.generatePoolParams(usdcTokenMint, usdcTokenMint, usdcFeedAccount, usdcPriceUpdate);
wooconfigAccount = fromPoolParams.wooconfig;
solWooracle = fromPoolParams.wooracle;
solPool = fromPoolParams.woopool;
solTokenVault = fromPoolParams.tokenVault;
usdcPool = toPoolParams.woopool;
usdcTokenVault = toPoolParams.tokenVault;
// 1. Deposit 20k USDC into USDC pool for swaps
console.log("2. Deposit 20k USDC into USDC pool for swaps");
const depositAmount = new BN(20000_000000);
let usdcAta = token.getAssociatedTokenAddressSync(usdcTokenMint, keypair.publicKey);
const tx2 = new anchor.web3.Transaction();
const amountVal = BigInt(depositAmount.toString()); // 20k USDC
tx2.add(
token.createMintToInstruction(usdcTokenMint, usdcAta, keypair.publicKey, amountVal)
);
await provider.sendAndConfirm(tx2, [keypair], {maxRetries: 5, commitment: "confirmed"})
// 20k USDC
const tx = await program
.methods
.deposit(depositAmount)
.accounts({
wooconfig: wooconfigAccount,
tokenMint: usdcTokenMint,
authority: keypair.publicKey,
tokenOwnerAccount: usdcAta,
woopool: usdcPool,
tokenVault: usdcTokenVault,
tokenProgram: token.TOKEN_PROGRAM_ID,
})
.signers([keypair])
.rpc(confirmOptionsRetryTres);
console.log("150 SOL Airdrop complete");
const solTransferTranscation = new Transaction().add(
// trasnfer SOL to WSOL into ata account
SystemProgram.transfer({
fromPubkey: fromWallet.publicKey,
toPubkey: solTokenAccount,
lamports: fromAmount,
}),
);
await provider.sendAndConfirm(solTransferTranscation, [fromWallet], { maxRetries: 5, commitment: "confirmed" });
console.log("SOL transferred");
const syncWsol = new Transaction().add(
// sync wrapped SOL balance
token.createSyncNativeInstruction(solTokenAccount)
);
await provider.sendAndConfirm(syncWsol, [], { maxRetries: 5, commitment: "confirmed" });
console.log("SOL converted to WSOL");
// step 3: Set Max Gamma.
console.log("3. Set Max Gamma to 5000000000000000 (same as ARB on arbitrum)");
// max gamma is set to 300 (impractically small value) on creation
// hence set max gamma to a meaningful value
// ARB on arbitrum has these values
// reserve uint192 : 307455046866289276248073
// feeRate uint16 : 25
// maxGamma uint128 : 5000000000000000
// maxNotionalSwap uint128 : 500000000000
await poolUtils.program
.methods
.setPoolMaxGamma(new BN(5000000000000000)) // same as ARB on arbitrum
.accounts({
wooconfig: wooconfigAccount,
woopool: solPool,
authority: provider.wallet.publicKey
}).rpc(confirmOptionsRetryTres);
// 4: initialize testPrice to max feasible price. bound is 2.5% same as Arbitrum
console.log("4. initialize testPrice to max feasible price. bound is 2.5% same as Arbitrum");
// set test price to max feasible price
const bound = new BN(24).mul(tenpow15);
const low_bound = pythoracle_price.mul(tenpow18.sub(bound)).div(tenpow18);
const upper_bound = pythoracle_price.mul(tenpow18.add(bound)).div(tenpow18);
testPrice = upper_bound;
// ---------------- Set up finish -----------
// test values:
console.log("\nTest values:");
console.log("test price:" + testPrice);
console.log("test spread:" + testSpread);
console.log("test coeff:" + testCoeff);
console.log("\nTest A: swap 100 SOL; 2 SOL at a time and show spread is not updated");
console.log("\nA.1 Set wooracle state to test values");
// set price to 0 to not trigger update_spread when setting the test price
await program
.methods
.setWooPrice(new BN(0))
.accounts({
wooconfig: wooconfigAccount,
wooracle: solWooracle,
authority: provider.wallet.publicKey,
})
.rpc(confirmOptionsRetryTres);
// Set the wooracle state to test values
await program
.methods
.setWooState(testPrice, testCoeff, testSpread)
.accounts({
wooconfig: wooconfigAccount,
wooracle: solWooracle,
authority: provider.wallet.publicKey,
})
.rpc(confirmOptionsRetryTres);
const wooracleBeforeSwap100 = await program.account.wooracle.fetch(solWooracle);
// Assert the price, spread and coeff are set to the test values
assert.ok(wooracleBeforeSwap100.price.eq(testPrice));
assert.ok(wooracleBeforeSwap100.spread.eq(testSpread));
assert.ok(wooracleBeforeSwap100.coeff.eq(testCoeff));
console.log("\nWooracle state before swaps:\n");
console.log(`price - ${wooracleBeforeSwap100.price}`);
console.log(`coeff - ${wooracleBeforeSwap100.coeff}`);
console.log(`spread - ${wooracleBeforeSwap100.spread}`);
const [fromPrice, fromFeasible] = await poolUtils.getOraclePriceResult(fromPoolParams.wooconfig, fromPoolParams.wooracle, solPriceUpdate, usdcPriceUpdate);
console.log(`swap price - ${fromPrice}`);
console.log(`swap feasible - ${fromFeasible}`);
// perform swap of 100 SOL; 2 SOL at a time
console.log("\nA.2 Swap 100 SOL; 2 SOL at a time");
console.log("\n Swapping 100 SOL; 2 SOL at a time to not trigger update to spread\n");
for (let i = 1; i <= 50; i++) {
await program
.methods
.swap(new BN(2 * LAMPORTS_PER_SOL), new BN(0))
.accounts({
wooconfig: fromPoolParams.wooconfig,
tokenProgram: token.TOKEN_PROGRAM_ID,
payer: fromWallet.publicKey, // is the user want to do swap
wooracleFrom: fromPoolParams.wooracle,
woopoolFrom: fromPoolParams.woopool,
tokenOwnerAccountFrom: solTokenAccount,
tokenVaultFrom: fromPoolParams.tokenVault,
priceUpdateFrom: solPriceUpdate,
wooracleTo: toPoolParams.wooracle,
woopoolTo: toPoolParams.woopool,
tokenOwnerAccountTo: usdcTokenAccount,
tokenVaultTo: toPoolParams.tokenVault,
priceUpdateTo: usdcPriceUpdate,
woopoolQuote: toPoolParams.woopool,
quotePriceUpdate: usdcPriceUpdate,
quoteTokenVault: toPoolParams.tokenVault,
rebateTo: fromWallet.publicKey,
})
.signers([fromWallet])
.rpc(confirmOptionsRetryTres);
}
console.log("Wooracle state after swapping 100 SOL\n");
const wooracleAfterSwap100 = await program.account.wooracle.fetch(solWooracle);
console.log(`price - ${wooracleAfterSwap100.price}`);
console.log(`coeff - ${wooracleAfterSwap100.coeff}`);
console.log(`spread - ${wooracleAfterSwap100.spread}`);
console.log("Price difference:" + wooracleBeforeSwap100.price.sub(wooracleAfterSwap100.price));
console.log("spread before swap of 100 SOL == spread after swap == " + testSpread);
console.log("\nA.3 Assert spread has not changed");
// assert spread has not changed
assert.ok(wooracleAfterSwap100.spread.eq(testSpread));
// reset the wooracle state to test values
console.log("\nTest B: Swap 5 SOL with the same wooracle state and show spread is updated");
console.log("\nB.1 Reset wooracle state to test values");
// set price to 0 to not trigger update_spread when setting the test price
await program
.methods
.setWooPrice(new BN(0))
.accounts({
wooconfig: wooconfigAccount,
wooracle: solWooracle,
authority: provider.wallet.publicKey,
})
.rpc(confirmOptionsRetryTres);
// Set the wooracle state to test values
await program
.methods
.setWooState(testPrice, testCoeff, testSpread)
.accounts({
wooconfig: wooconfigAccount,
wooracle: solWooracle,
authority: provider.wallet.publicKey,
})
.rpc(confirmOptionsRetryTres);
const wooracleBeforeSwap5 = await program.account.wooracle.fetch(solWooracle);
// Assert the price, spread and coeff are set to the test values
assert.ok(wooracleBeforeSwap5.price.eq(testPrice));
assert.ok(wooracleBeforeSwap5.spread.eq(testSpread));
assert.ok(wooracleBeforeSwap5.coeff.eq(testCoeff));
console.log(`spread before swap of 5 sol == test spread == ${wooracleBeforeSwap5.spread}`);
// swap 5 SOL
console.log("\nB.2 Swap 5 SOL");
await program
.methods
.swap(new BN(5 * LAMPORTS_PER_SOL), new BN(0))
.accounts({
wooconfig: fromPoolParams.wooconfig,
tokenProgram: token.TOKEN_PROGRAM_ID,
payer: fromWallet.publicKey, // is the user want to do swap
wooracleFrom: fromPoolParams.wooracle,
woopoolFrom: fromPoolParams.woopool,
tokenOwnerAccountFrom: solTokenAccount,
tokenVaultFrom: fromPoolParams.tokenVault,
priceUpdateFrom: solPriceUpdate,
wooracleTo: toPoolParams.wooracle,
woopoolTo: toPoolParams.woopool,
tokenOwnerAccountTo: usdcTokenAccount,
tokenVaultTo: toPoolParams.tokenVault,
priceUpdateTo: usdcPriceUpdate,
woopoolQuote: toPoolParams.woopool,
quotePriceUpdate: usdcPriceUpdate,
quoteTokenVault: toPoolParams.tokenVault,
rebateTo: fromWallet.publicKey,
})
.signers([fromWallet])
.rpc(confirmOptionsRetryTres);
console.log("Wooracle state after swapping 5 SOL\n");
const wooracleAfterSwap5 = await program.account.wooracle.fetch(solWooracle);
console.log(`price - ${wooracleAfterSwap5.price}`);
console.log(`coeff - ${wooracleAfterSwap5.coeff}`);
console.log(`spread - ${wooracleAfterSwap5.spread}`);
console.log("\nPrice difference:" + wooracleBeforeSwap5.price.sub(wooracleAfterSwap5.price));
console.log("\nspread before swap of 5 SOL != spread after swap of 5 SOL:", wooracleBeforeSwap5.spread != wooracleAfterSwap5.spread);
console.log("\nB.3 Assert spread has changed after the swap of 5 SOL");
assert.ok(!wooracleAfterSwap5.spread.eq(testSpread));
});
});
});
running anchor test
results in the following output:
woofi_spread
#test_spread_not_updated
solPrice Price {
conf: '9077400',
expo: -8,
price: '14233670000',
publishTime: 1728146557
}
usdcPrice Price {
conf: '111040',
expo: -8,
price: '100002593',
publishTime: 1728146557
}
pythoracle_price:14233300930
pythoracle_decimal:8
updated at - Sat Oct 05 2024 22:12:37 GMT+0530
1. Mint 150 SOL to from wallet
2. Deposit 20k USDC into USDC pool for swaps
150 SOL Airdrop complete
SOL transferred
SOL converted to WSOL
3. Set Max Gamma to 5000000000000000 (same as ARB on arbitrum)
4. initialize testPrice to max feasible price. bound is 2.5% same as Arbitrum
Test values:
test price:14574900152
test spread:500000000000000
test coeff:3000000000000
Test A: swap 100 SOL; 2 SOL at a time and show spread is not updated
A.1 Set wooracle state to test values
Wooracle state before swaps:
price - 14574900152
coeff - 3000000000000
spread - 500000000000000
swap price - 14574900152
swap feasible - 1
A.2 Swap 100 SOL; 2 SOL at a time
Swapping 100 SOL; 2 SOL at a time to not trigger update to spread
Wooracle state after swapping 100 SOL
price - 13963813321
coeff - 3000000000000
spread - 500000000000000
Price difference:611086831
spread before swap of 100 SOL == spread after swap == 500000000000000
A.3 Assert spread has not changed
Test B: Swap 5 SOL with the same wooracle state and show spread is updated
B.1 Reset wooracle state to test values
spread before swap of 5 sol == test spread == 500000000000000
B.2 Swap 5 SOL
Wooracle state after swapping 5 SOL
price - 14543035994
coeff - 3000000000000
spread - 1687078619323403
Price difference:31864158
spread before swap of 5 SOL != spread after swap of 5 SOL: true
B.3 Assert spread has changed after the swap of 5 SOL
✔ fails to update spread (29992ms)
The graph here uses the values used for the PoC:
Test values:
price = 145.74
coeff = c =0.000003
spread = 0.0005
It can be seen in the graph that the spread is only updated if the swap value > 2.28
SOL (intersection point of red and black graphs).
The PoC swaps 2 SOL at a time to not trigger spread update while also changing the price. After swapping 100 SOL, the SOL token price comes down by 6.1
without triggering an update to the spread.
The PoC also shows that the same wooracle state would result in update to spread if the swap size is 5 SOL (> 2.28).
i.e For the same initial wooracle state:
for base-to-quote swap, spread is updated if and only if
Gamma = G = B * p * k
new price = p1 = p * (1 - G)
Because 0 <= G < 1 => p1 < p and min_p = p1, max_p = p
price_decrease = p - p1
anti_s = l = (p1 / p) * (1 / (1-s))
new_s = 1 - l
G
is the rate of price change. After the swap, price decreases by G * p
.
new_s =
$$ 1 - (\frac{p1}{p} * \frac{1}{1-s}) $$
$$ = 1 - ((1 - G) * \frac{1}{1-s}) $$
$$ = \frac{G - s}{1 - s} $$
spread is not updated if
$$ s > \frac{G - s}{1 - s} $$
$G$ solely depends on the coefficient, base amount and the price. Base amount can be chosen to meet the above condition to not update the spread.
Intuitively, In the issue H.1 of the sherlock report, max gamma served as the upper bound for the max change in the price. The exploit used swaps of smaller amounts such that gamma is less than the max gamma.
After the fix (current implementation), the upper bound for max change (i.e gamma) in the price has changed. It is the above mentioned condition to not trigger spread update. Same exploit with smaller amounts such that gamma does not satisfy the above condition can be used. This is the issue described here
One other way of thinking about the issue is
Because only the price change between the most recent price and the current price is considered, it might not always meet the required threshold. A swap could be divided into two swaps executed at price p1, p2 and current price p3. The price changes (p2 - p1), (p3 - p2) might not meet the threshold individually, but (p3 - p1) might meet the threshold.
The code should either consider change of price for a certain period instead of just the previous and the current. Or spread update should not require a threshold. It should be a continuous function.
S3v3ru5
High
Dividing large swaps into smaller swaps does not update spread
Summary
The WooFi team has updated the
wooracle
to update the spread for new price. This is a fix for many issues reported in the past audits of WooFi Solodity .The fix is incomplete as user can divide the large swap into multiple smaller swaps such that the condition for updating the spread is not satisfied without any additional costs.
Vulnerability details
The new spread is calculated as
https://github.com/sherlock-audit/2024-08-woofi-solana-deployment/blob/main/WOOFi_Solana/programs/woofi/src/state/wooracle.rs#L151-L176
Consider the case of Base-to-Quote swap
p1 < p
andmin_p = p1
,max_p
=p
p - p1
spread
is update ifnew_s > s
Graphing the equations here with
x = B = number of base tokens swapped
without any decimals i.e Ifx = 2
then that means 2 SOL are swapped not 2 lamports.y = new_s
, depends onp1
which changes based onx = B
y = s
, a constant graphp1 - p
based on amount of tokens swapped.The WooFi program only updates the spread if
new_s > s
i.e where "black graph" is above the "red graph". Its easy to see that price can be decreased significantly without affecting the spread.In the example, chosen randomly, shown in the graph:
The red and black graphs meet at x = 5.3 i.e User can swap upto 5.3 base tokens and not cause an update to the spread. The price decreased upon 5.3 tokens is
3
.The User can repeat the process multiple times without additional costs because:
This issue makes the implemented fix incomplete.
Root Cause
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
Price manipulation has described in Issue H-1 of the Sherlock audit report. For the attack to be profitable Pyth price also needs to be inaccurate because of external conditions.
User can divide large swaps into smaller swaps. The net effect of amount swapped is same in both cases, however spread is not updated when smaller swaps are performed. This leads to loss of more amount to arbitage as described in Issue 3.1 of the zellic report.
Sandwich attack can be used as described in issue 3.6 of the Zellic report.
Impact
Because WooFi simulates centralized exchange, one of invariants it needs to ensure is to update spread upon large swaps. The user can divide into smaller swaps without additional costs. As a result of which, the spread is not updated even after large amount is swapped.
Impact noted in Issue 3.1 of the Zellic report:
PoC
No response
Mitigation