check struct_deconstruct has fn ret values as input

src/detectors/unused_return.rs

@@ -38,6 +38,8 @@ impl Detector for UnusedReturn {
             for f in compilation_unit.functions_user_defined() {
                 for (i, stmt) in f.get_statements().iter().enumerate() {
                     if let SierraStatement::Invocation(invoc) = stmt {
+                        // Get the return values from the function
+                        let ret_vars = &invoc.branches[0].results;
                         // Get the concrete libfunc called
                         let libfunc = compilation_unit
                             .registry()
@@ -83,7 +85,8 @@ impl Detector for UnusedReturn {
                                     .registry()
                                     .get_libfunc(&invoc.libfunc_id)
                                     .expect("Library function not found in the registry");
-
+                                // Get the parameters to the instruction for the struct_deconstruct case
+                                let args = &invoc.args;
                                 // Immediate Drop instruction
                                 if let CoreConcreteLibfunc::Drop(drop_libfunc) = libfunc {
                                     let ty_dropped = compilation_unit
@@ -117,23 +120,24 @@ impl Detector for UnusedReturn {
                                             .registry()
                                             .get_libfunc(&invoc.libfunc_id)
                                             .expect("Library function not found in the registry");
-
-                                        self.iterate_struct_deconstruct(
-                                            compilation_unit,
-                                            &mut results,
-                                            libfunc,
-                                            stmt_to_check,
-                                            stmt,
-                                            &f.name(),
-                                            return_variables,
-                                        );
+                                        // We want to make sure the struct_deconstruct corresponds to the function's return values, and not any misc. struct cleanup
+                                        if ret_vars.contains(&args[0]) {
+                                            self.iterate_struct_deconstruct(
+                                                compilation_unit,
+                                                &mut results,
+                                                libfunc,
+                                                stmt_to_check,
+                                                stmt,
+                                                &f.name(),
+                                                return_variables,
+                                            );
+                                        }
                                     }
                                 } else if let CoreConcreteLibfunc::Enum(
                                     EnumConcreteLibfunc::Match(_),
                                 ) = libfunc
                                 {
                                     let return_variables = invoc.branches[0].results.len();
-
                                     // Jump one statement which is a branch_align and the next one will be a struct_deconstruct
                                     let stmt_to_check = &following_stmts[2..];
                                     if let SierraStatement::Invocation(invoc) = &stmt_to_check[0] {

tests/detectors/unused_return.cairo

@@ -1,11 +1,17 @@
 use option::OptionTrait;
-
+use serde::Serde;
 #[starknet::contract]
 mod UnusedReturn {
     #[storage]
     struct Storage {
         value: felt252,
     }
+    #[derive(Copy,Drop,Serde)]
+    struct TestStruct {
+        val1: felt252,
+        val2: felt252,
+        val3: felt252
+    }
 
     #[external(v0)]
     fn unused_return_1(ref self: ContractState, amount: felt252) {
@@ -15,7 +21,7 @@ mod UnusedReturn {
     #[external(v0)]
     fn unused_return_2(ref self: ContractState, amount: felt252) -> felt252 {
         let (a,b,d) = f_2(amount);
-        let c = a  ;
+        let c = a;
         a
     }
 
@@ -31,7 +37,17 @@ mod UnusedReturn {
 
     #[external(v0)]
     fn unused_return_5(ref self: ContractState) {
-        let a = f_5(ref self);        
+        let a = f_5(ref self);
+    }
+
+    #[external(v0)]
+    fn unused_return_6(ref self: ContractState, s: TestStruct) {
+        let a = f_6(ref self, s.val1,s.val2);
+    }
+
+    #[external(v0)]
+    fn unused_return_7(ref self: ContractState, s: TestStruct) {
+        let a = f_7(ref self, s);
     }
 
     #[external(v0)]
@@ -50,6 +66,18 @@ mod UnusedReturn {
         f_5(ref self)
     }
 
+    #[external(v0)]
+    fn no_report4(ref self: ContractState, s: TestStruct) -> felt252 {
+        let a = f_6(ref self, s.val1,s.val2);
+        s.val3 + a
+    }
+
+    #[external(v0)]
+    fn no_report5(ref self: ContractState, s: TestStruct) -> felt252 {
+        let a = f_6(ref self, s.val1,s.val2);
+        a
+    }
+
     fn f_1(ref self: ContractState, amount: felt252) -> felt252 {
         self.value.write(amount);
         23
@@ -72,4 +100,15 @@ mod UnusedReturn {
         a * 2
     }
 
-}
+    fn f_6(ref self:ContractState, amount1: felt252, amount2: felt252) -> felt252 {
+        let a = self.value.read();
+        let ret = amount1 * amount2;
+        ret + a
+    }
+
+    fn f_7(ref self:ContractState, s:TestStruct) -> felt252 {
+        let a = self.value.read();
+        s.val1 * s.val2 * s.val3 + a
+    }
+
+}

tests/snapshots/integration_tests__detectors@controlled_library_call.cairo.snap

@@ -10,10 +10,4 @@ input_file: tests/detectors/controlled_library_call.cairo
         confidence: Medium,
         message: "Library call to user controlled class hash in controlled_library_call::controlled_library_call::TestContract::bad1\n function_call<user@controlled_library_call::controlled_library_call::IAnotherContractLibraryDispatcherImpl::foo>([11], [12], [13], [14], [15]) -> ([7], [8], [9], [10])",
     },
-    Result {
-        impact: High,
-        name: "controlled-library-call",
-        confidence: Medium,
-        message: "Library call to user controlled class hash in controlled_library_call::controlled_library_call::TestContract::internal_lib_call\n function_call<user@controlled_library_call::controlled_library_call::IAnotherContractLibraryDispatcherImpl::foo>([10], [11], [12], [13], [14]) -> ([6], [7], [8], [9])",
-    },
 ]

tests/snapshots/integration_tests__detectors@dead_code.cairo.snap

@@ -10,16 +10,4 @@ input_file: tests/detectors/dead_code.cairo
         confidence: Medium,
         message: "The function dead_code::dead_code::DeadCode::add_1 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow/underflow safe",
     },
-    Result {
-        impact: Medium,
-        name: "felt252-unsafe-arithmetic",
-        confidence: Medium,
-        message: "The function dead_code::dead_code::DeadCode::add_2 uses the felt252 operation felt252_add([0], [1]) -> ([2]), which is not overflow/underflow safe",
-    },
-    Result {
-        impact: Low,
-        name: "dead-code",
-        confidence: Medium,
-        message: "Function add_2 defined in dead_code::dead_code::DeadCode is never used",
-    },
 ]

tests/snapshots/integration_tests__detectors@unused_arguments.cairo.snap

@@ -4,22 +4,10 @@ expression: results
 input_file: tests/detectors/unused_arguments.cairo
 ---
 [
-    Result {
-        impact: Low,
-        name: "unused-arguments",
-        confidence: Medium,
-        message: "The 2nd argument in unused_arguments::unused_arguments::UnusedArguments::unused_2 is never used",
-    },
     Result {
         impact: Low,
         name: "unused-arguments",
         confidence: Medium,
         message: "The 3rd argument in unused_arguments::unused_arguments::UnusedArguments::unused_1 is never used",
     },
-    Result {
-        impact: Low,
-        name: "unused-arguments",
-        confidence: Medium,
-        message: "The 3rd argument in unused_arguments::unused_arguments::UnusedArguments::unused_2 is never used",
-    },
 ]

tests/snapshots/integration_tests__detectors@unused_return.cairo.snap

@@ -10,6 +10,42 @@ input_file: tests/detectors/unused_return.cairo
         confidence: Medium,
         message: "The function unused_return::unused_return::UnusedReturn::f_5 uses the felt252 operation felt252_mul([14], [15]) -> ([16]), which is not overflow/underflow safe",
     },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::f_6 uses the felt252 operation felt252_add([16], [17]) -> ([18]) with the user-controlled parameters: [16], which is not overflow/underflow safe",
+    },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::f_6 uses the felt252 operation felt252_mul([3], [4]) -> ([16]) with the user-controlled parameters: [3],[4], which is not overflow/underflow safe",
+    },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::f_7 uses the felt252 operation felt252_add([19], [20]) -> ([21]) with the user-controlled parameters: [19], which is not overflow/underflow safe",
+    },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::f_7 uses the felt252 operation felt252_mul([15], [16]) -> ([18]) with the user-controlled parameters: [15],[16], which is not overflow/underflow safe",
+    },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::f_7 uses the felt252 operation felt252_mul([18], [17]) -> ([19]) with the user-controlled parameters: [18],[17], which is not overflow/underflow safe",
+    },
+    Result {
+        impact: Medium,
+        name: "felt252-unsafe-arithmetic",
+        confidence: Medium,
+        message: "The function unused_return::unused_return::UnusedReturn::no_report4 uses the felt252 operation felt252_add([6], [18]) -> ([19]) with the user-controlled parameters: [6],[18], which is not overflow/underflow safe",
+    },
     Result {
         impact: Medium,
         name: "unused-return",
@@ -40,4 +76,16 @@ input_file: tests/detectors/unused_return.cairo
         confidence: Medium,
         message: "Return value unused for the function call function_call<user@unused_return::unused_return::UnusedReturn::f_5>([6], [7], [8]) -> ([3], [4], [5]) in unused_return::unused_return::UnusedReturn::unused_return_5",
     },
+    Result {
+        impact: Medium,
+        name: "unused-return",
+        confidence: Medium,
+        message: "Return value unused for the function call function_call<user@unused_return::unused_return::UnusedReturn::f_6>([10], [11], [12], [13], [14]) -> ([7], [8], [9]) in unused_return::unused_return::UnusedReturn::unused_return_6",
+    },
+    Result {
+        impact: Medium,
+        name: "unused-return",
+        confidence: Medium,
+        message: "Return value unused for the function call function_call<user@unused_return::unused_return::UnusedReturn::f_7>([7], [8], [9], [10]) -> ([4], [5], [6]) in unused_return::unused_return::UnusedReturn::unused_return_7",
+    },
 ]