Skip to content

Commit

Permalink
fix ONFT double counting vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
NoahSaso committed Aug 2, 2024
1 parent 5b6093e commit d4d085e
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
6 changes: 5 additions & 1 deletion contracts/voting/dao-voting-onft-staked/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,14 @@ pub fn execute_confirm_stake(
deps: DepsMut,
env: Env,
info: MessageInfo,
token_ids: Vec<String>,
mut token_ids: Vec<String>,
) -> Result<Response, ContractError> {
let config = CONFIG.load(deps.storage)?;

// de-duplicate token IDs to prevent double-counting exploit
token_ids.sort();
token_ids.dedup();

// verify sender prepared and transferred all the tokens
let sender_prepared_all = token_ids
.iter()
Expand Down
19 changes: 19 additions & 0 deletions contracts/voting/dao-voting-onft-staked/src/testing/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,25 @@ fn test_stake_tokens() -> anyhow::Result<()> {
assert_eq!(total, Uint128::new(1));
assert_eq!(personal, Uint128::new(1));

// Registering duplicate token IDs does not provide more voting power.
mint_nft(&mut app, &nft, STAKER, "2")?;
prepare_stake_nft(&mut app, &module, STAKER, "2")?;
send_nft(&mut app, &nft, "2", STAKER, module.as_str())?;
app.execute_contract(
Addr::unchecked(STAKER),
module.clone(),
&ExecuteMsg::ConfirmStake {
token_ids: vec!["2".to_string(), "2".to_string(), "2".to_string()],
},
&[],
)?;

app.update_block(next_block);

let (total, personal) = query_total_and_voting_power(&app, &module, STAKER, None)?;
assert_eq!(total, Uint128::new(2));
assert_eq!(personal, Uint128::new(2));

Ok(())
}

Expand Down

0 comments on commit d4d085e

Please sign in to comment.