[audit] 3. Users can inflate their voting power by registering duplicate NFT token IDs

contracts/external/cw-tokenfactory-issuer/tests/mod.rs

@@ -1,7 +1,3 @@
-// Ignore integration tests for code coverage since there will be problems with dynamic linking libosmosistesttube
-// and also, tarpaulin will not be able read coverage out of wasm binary anyway
-#![cfg(not(tarpaulin))]
-
 #[cfg(feature = "test-tube")]
 mod cases;
 #[cfg(feature = "test-tube")]

contracts/external/dao-migrator/README.md

@@ -5,7 +5,7 @@
 
 Here is the [discussion](https://github.com/DA0-DA0/dao-contracts/discussions/607).
 
-A migrator module for a DAO DAO DAO which handles migration for DAO modules 
+A migrator module for a DAO DAO DAO which handles migration for DAO modules
 and test it went successfully.
 
 DAO core migration is handled by a proposal, which adds this module and do
@@ -14,6 +14,7 @@ If custom module is found, this TX fails and migration is cancelled, custom
 module requires a custom migration to be done by the DAO.
 
 # General idea
+
 1. Proposal is made to migrate DAO core to V2, which also adds this module to the DAO.
 2. On init of this contract, a callback is fired to do the migration.
 3. Then we check to make sure the DAO doesn't have custom modules.
@@ -23,9 +24,10 @@ module requires a custom migration to be done by the DAO.
 7. In any case where 1 migration fails, we fail the whole TX.
 
 # Important notes
-* custom modules cannot reliably be migrated by this contract, 
-because of that we fail the process to avoid any unwanted results.
 
-* If any module migration fails we fail the whole thing, 
-this is to make sure that we either have a fully working V2,
-or we do nothing and make sure the DAO is operational at any time.
+- custom modules cannot reliably be migrated by this contract,
+  because of that we fail the process to avoid any unwanted results.
+
+- If any module migration fails we fail the whole thing,
+  this is to make sure that we either have a fully working V2,
+  or we do nothing and make sure the DAO is operational at any time.

contracts/proposal/dao-proposal-condorcet/src/testing/suite.rs

@@ -146,7 +146,7 @@ impl SuiteBuilder {
         if let Some(candidates) = self.with_proposal {
             suite
                 .propose(
-                    &suite.sender(),
+                    suite.sender(),
                     (0..candidates)
                         .map(|_| vec![unimportant_message()])
                         .collect(),

contracts/voting/dao-voting-onft-staked/src/contract.rs

@@ -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()

contracts/voting/dao-voting-onft-staked/src/testing/tests.rs

@@ -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(())
 }
 

contracts/voting/dao-voting-token-staked/src/tests/test_tube/mod.rs

@@ -1,6 +1,2 @@
-// Ignore integration tests for code coverage since there will be problems with dynamic linking libosmosistesttube
-// and also, tarpaulin will not be able read coverage out of wasm binary anyway
-#![cfg(not(tarpaulin))]
-
 mod integration_tests;
 mod test_env;

packages/dao-testing/src/test_tube/mod.rs

@@ -1,7 +1,3 @@
-// Ignore integration tests for code coverage since there will be problems with dynamic linking libosmosistesttube
-// and also, tarpaulin will not be able read coverage out of wasm binary anyway
-#![cfg(not(tarpaulin))]
-
 // Integrationg tests using an actual chain binary, requires
 // the "test-tube" feature to be enabled
 // cargo test --features test-tube