chore: merge PR #38 from BlockheaderWeb3-Community/security

test: validate attacker counter count logic

src/aggregator.cairo

@@ -24,17 +24,22 @@ mod Aggregator {
     use super::{IAggregator};
     use starknet::{ContractAddress};
 
-    
+
     #[storage]
     struct Storage {
         aggr_count: u32,
         aggr_owner: ContractAddress,
         ownable_addr: ContractAddress,
-        counter_addr: ContractAddress, 
+        counter_addr: ContractAddress,
     }
 
     #[constructor]
-    fn constructor(ref self: ContractState, owner_addr: ContractAddress, ownable_addr: ContractAddress, counter_addr: ContractAddress,) {
+    fn constructor(
+        ref self: ContractState,
+        owner_addr: ContractAddress,
+        ownable_addr: ContractAddress,
+        counter_addr: ContractAddress,
+    ) {
         self.aggr_owner.write(owner_addr);
         self.ownable_addr.write(ownable_addr);
         self.counter_addr.write(counter_addr);
@@ -44,36 +49,28 @@ mod Aggregator {
     impl AggrImpl of IAggregator<ContractState> {
         // references Ownable contract methods
         // returns the address of the ownable contract
-        fn get_ownable_owner(
-            self: @ContractState, 
-        ) -> ContractAddress {
-            IOwnableDispatcher { contract_address: self.ownable_addr.read()}.get_owner()
+        fn get_ownable_owner(self: @ContractState,) -> ContractAddress {
+            IOwnableDispatcher { contract_address: self.ownable_addr.read() }.get_owner()
         }
 
-        fn set_ownable_owner(
-            ref self: ContractState, new_owner: ContractAddress
-        ) {
-            IOwnableDispatcher { contract_address: self.ownable_addr.read()}.set_owner(new_owner);
+        fn set_ownable_owner(ref self: ContractState, new_owner: ContractAddress) {
+            IOwnableDispatcher { contract_address: self.ownable_addr.read() }.set_owner(new_owner);
         }
 
         // references Counter contract methods
         fn get_counter_count(self: @ContractState) -> u32 {
-            ICounterDispatcher { contract_address: self.counter_addr.read()}.get_count()
+            ICounterDispatcher { contract_address: self.counter_addr.read() }.get_count()
         }
 
-        fn set_counter_count(
-            ref self: ContractState, amount: u32
-        ) {
-            ICounterDispatcher { contract_address: self.counter_addr.read()}.set_count(amount);
+        fn set_counter_count(ref self: ContractState, amount: u32) {
+            ICounterDispatcher { contract_address: self.counter_addr.read() }.set_count(amount);
         }
 
         fn get_aggr_owner(self: @ContractState) -> ContractAddress {
             self.aggr_owner.read()
         }
     }
 }
-
 // 0x1dd6e81d875e3451d14d418af0f42464bbaec19127465e700fb07cb403eb4cc - ca
 
 
-

src/attack_counter.cairo

@@ -1,7 +1,9 @@
 // AttackCounter Contract
-// Whereas a function is considered as a read-only function if its self is a snapshot of storage as depicted thus (self: @TContractState), it is not true that such a functin cannot modify state
-// By leveraging syscalls like `call_contract_syscall`, such function can modify state
-// In this contract, we showcase this possibility of using `call_contract_syscall` in our AttackCounter contract to modify the `count` state of our simple counter contract
+// Whereas a function is considered as a read-only function if its self is a snapshot of storage as
+// depicted thus (self: @TContractState), it is not true that such a functin cannot modify state By
+// leveraging syscalls like `call_contract_syscall`, such function can modify state In this
+// contract, we showcase this possibility of using `call_contract_syscall` in our AttackCounter
+// contract to modify the `count` state of our simple counter contract
 
 #[starknet::interface]
 pub trait IAttackCounter<TContractState> {
@@ -10,14 +12,15 @@ pub trait IAttackCounter<TContractState> {
     fn counter_count(self: @TContractState) -> u32;
 
     // set count
-    fn attack_count(self: @TContractState);
+    fn attack_count(self: @TContractState, amount: u32);
 }
 
 
 #[starknet::contract]
 pub mod AttackCounter {
-    use crate::counter::{ ICounterDispatcher, ICounterDispatcherTrait};
-    use starknet::{ContractAddress, syscalls:: call_contract_syscall};
+    use super::IAttackCounter;
+use crate::counter::{ICounterDispatcher, ICounterDispatcherTrait};
+    use starknet::{ContractAddress, syscalls::call_contract_syscall};
     #[storage]
     struct Storage {
         counter_address: ContractAddress
@@ -35,14 +38,14 @@ pub mod AttackCounter {
             ICounterDispatcher { contract_address: counter_addr }.get_count()
         }
 
-        fn attack_count(self: @ContractState) {
+        fn attack_count(self: @ContractState, amount: u32) {
             let counter_addr = self.counter_address.read();
+            // let mut counter_current_count: u32 = self.counter_count();
             let selector = selector!("set_count");
 
             let mut args: Array<felt252> = array![];
-            let value = 100;
-            value.serialize(ref args);
+            amount.serialize(ref args);
             call_contract_syscall(counter_addr, selector, args.span());
         }
     }
-}
+}

src/counter.cairo

@@ -30,5 +30,7 @@ mod Counter {
 }
 // 0x0726aae552474f128529c982392e8490c496395c1a2d7879c029203ad12e97bb
 
-//__________________________________TODAY_______________________ 
-// 0x45f8e8b3d6ecf220d78fdc13a523ae8ecaa90581ee68baa958d8ba3181841e9 - counter ca
+//__________________________________TODAY_______________________
+// 0x45f8e8b3d6ecf220d78fdc13a523ae8ecaa90581ee68baa958d8ba3181841e9 - counter ca
+
+

src/ownable.cairo

@@ -47,9 +47,9 @@ mod Ownable {
 
 // 0x6331d7d1cb9bc762785d083570d0d594fcf57cf3e5384209b59435c3f7e6d8b -- justice
 
-
-
-//__________________________________TODAY_______________________ 
+//__________________________________TODAY_______________________
 // 0xdedef0be763547e8e505d12fac321d0de4e9bd51635ac5fa00ae61d12e463e
 // 0x6f2f6eb269f9741d5bb9cb633bfb632a0d71e0622b195ef4c4e66e8f1fee9fe - deploy_dev account
-// 0x2a601649affa4fb870f919058baeed96729b1d7be7282b978e5ba50852d7c77 - ownable ca
+// 0x2a601649affa4fb870f919058baeed96729b1d7be7282b978e5ba50852d7c77 - ownable ca
+
+

src/vulnerable_stake.cairo

@@ -18,7 +18,7 @@ pub struct StakerInfo {
 
 
 #[starknet::contract]
-mod VulnerableStake {
+pub mod VulnerableStake {
     use starknet::{ContractAddress, get_caller_address};
     use starknet::storage::{StoragePointerWriteAccess, StoragePathEntry, Map};
     use super::{StakerInfo, IVulnerableStake,};

src/vulnerable_token.cairo

@@ -1,7 +1,10 @@
-// NB 
+// NB
 // this is not an ERC-20 token contract as it doesn't implement the ERC-20 standard
-// This minimalistic VulnerableToken contract is a token contract whose `mint_token` method lacks the requisite access control to for the execution of critical operations like minting/burning of tokens; in our case here, we are targetting the increase total supply operation of the contract.
-// It is being used here to showcase the security vulnerability of exposing critical functions without access control
+// This minimalistic VulnerableToken contract is a token contract whose `mint_token` method lacks
+// the requisite access control to for the execution of critical operations like minting/burning of
+// tokens; in our case here, we are targetting the increase total supply operation of the contract.
+// It is being used here to showcase the security vulnerability of exposing critical functions
+// without access control
 
 #[starknet::interface]
 pub trait IVulnerableToken<T> {
@@ -10,7 +13,7 @@ pub trait IVulnerableToken<T> {
 }
 
 #[starknet::contract]
-mod VulnerableToken {
+pub mod VulnerableToken {
     const MINT_AMOUNT: u256 = 1000;
 
     #[storage]

tests/test_attack_counter.cairo

@@ -0,0 +1,64 @@
+use snforge_std::{declare, ContractClassTrait, DeclareResultTrait};
+use starknet::{ContractAddress};
+use cairo_bootcamp_3::{
+    counter::{ICounterDispatcher, ICounterDispatcherTrait},
+    attack_counter::{IAttackCounterDispatcher, IAttackCounterDispatcherTrait}
+};
+
+// Deploys the given contract and returns the corresponding contract address
+fn deploy_util(contract_name: ByteArray, constructor_calldata: Array<felt252>) -> ContractAddress {
+    let contract = declare(contract_name).unwrap().contract_class();
+    let (contract_address, _) = contract.deploy(@constructor_calldata).unwrap();
+    contract_address
+}
+
+
+// Instruct snforge to only run this function when the `snforge test` command is used
+#[test]
+fn test_can_attacker_counter_count() {
+    // Deploy Counter contract and get the contract address
+
+    let mut counter_calldata: Array<felt252> = array![];
+
+    let counter_contract_address: ContractAddress = deploy_util("Counter", counter_calldata);
+
+    // Get an instance of the deployed Counter contract
+    let counter_instance = ICounterDispatcher { contract_address: counter_contract_address };
+
+    // test that Counter contract's count value = 0
+    // Get the initial value of count from storage
+    let count_1 = counter_instance.get_count();
+    // Assert that the inital count value is what is 0
+    assert_eq!(count_1, 0);
+
+    // setup AttackCounter contract calldata
+    let mut attacker_calldata: Array<felt252> = array![];
+    counter_contract_address.serialize(ref attacker_calldata);
+
+    // deploy AttackCounter contract
+    let attack_counter_address: ContractAddress = deploy_util("AttackCounter", attacker_calldata);
+
+    // get AttackCounter contract instance
+    let attacker_instance = IAttackCounterDispatcher { contract_address: attack_counter_address };
+
+    // call attack count to modify Counter contract's count state value to 100
+    attacker_instance.attack_count(100);
+
+    // fetch Counter's 2nd count value
+    let count_2 = counter_instance.get_count();
+    println!("count 2____{}", count_2);
+    // Assert that the 2nd count value was successfully set to 100
+    assert_eq!(count_2, 100);
+
+    // assert that Attacker's counter_count matches the counter's count state variable
+    assert_eq!(count_2, attacker_instance.counter_count());
+
+    // call attack count to modify Counter contract's count state value to 200
+     attacker_instance.attack_count(100);
+
+      // fetch Counter's 2nd count value
+    let count_3 = counter_instance.get_count();
+    println!("count 3____{}", count_3);
+    // Assert that the 3rd count value was successfully set to 200
+    assert_eq!(count_3, 200);
+}