Fix LGTM Errors (#1656)

* py/missing-call-to-init

* Remove deprecated should_profile argument

* WASM __init__ call and variable names

* Mute warnings about printing evm account IDs

LGTM thinks we're logging potentially sensitive data when we create a symbolic account and print the user ID. I can't imagine a context where we would ever care about this alert in general, so I'm going to disable it.

* Fix mixed attribute/function names

* Another missing __init__ call

* Remove single-op form of _bitwise_instruction

It's never used

* Use built-in ops instead of lambda

* Missing import

* Some unreachable code

Still missing some in evm.py

* Blacken

* Tag examples as examples

* NotImplemented -> NotImplementedError

* Fix unnecessary `pass`

* Revert "Remove single-op form of _bitwise_instruction"

This reverts commit 1b7a411.

* Mute overzealous call argument error

Co-authored-by: Brad Larsen <brad.larsen@trailofbits.com>

lgtm.yml

@@ -0,0 +1,6 @@
+queries:
+  - exclude: py/clear-text-logging-sensitive-data
+
+path_classifiers:
+  examples:
+    - examples/

manticore/core/plugin.py

@@ -129,7 +129,6 @@ def will_write_memory_callback(self, state, where, value, size):
     def did_write_memory_callback(self, state, where, value, size):
         if self.current_pc == where:
             raise Exception
-            return
 
         entry = {"type": "mem_write", "where": where, "value": value, "size": size}
         state.context.setdefault(self.context_key, []).append(entry)

manticore/core/smtlib/constraints.py

@@ -326,7 +326,7 @@ def migrate(self, expression, name_migration_map=None):
                         name=migrated_name,
                     ).array
                 else:
-                    raise NotImplemented(
+                    raise NotImplementedError(
                         f"Unknown expression type {type(foreign_var)} encountered during expression migration"
                     )
                 # Update the var to var mapping

manticore/core/smtlib/solver.py

@@ -274,7 +274,6 @@ def __del__(self):
             # self._proc.wait()
         except Exception as e:
             logger.error(str(e))
-            pass
 
     def _reset(self, constraints=None):
         """Auxiliary method to reset the smtlib external solver to initial defaults"""

manticore/core/state.py

@@ -61,15 +61,15 @@ class SerializeState(Concretize):
         disk so that analysis can later be resumed from this point.
     """
 
-    def setstate(self, state, _value):
+    def _setstate(self, state, _value):
         with open(self.filename, "wb") as statef:
             PickleSerializer().serialize(state, statef)
 
     def __init__(self, filename, **kwargs):
         super().__init__(
             f"Saving state to {filename}",
             BitVecConstant(32, 0),
-            setstate=self.setstate,
+            setstate=self._setstate,
             policy="ONE",
             **kwargs,
         )

manticore/ethereum/abi.py

@@ -264,7 +264,7 @@ def _deserialize(ty, buf: typing.Union[bytearray, bytes, Array], offset=0):
                 result.append(ABI._deserialize(ty[2], buf, dyn_offset))
                 dyn_offset += ty_size
         else:
-            raise NotImplemented
+            raise NotImplementedError(f"Could not deserialize type: {ty[0]}")
 
         return result
 

manticore/ethereum/manticore.py

@@ -513,7 +513,7 @@ def _make_symbolic_arguments(self, ty):
             for _ in range(rep):
                 result.append(self._make_symbolic_arguments(ty[2]))
         else:
-            raise NotImplemented
+            raise NotImplementedError(f"Could not produce symbolic argument of type {ty[0]}")
 
         return result
 

manticore/native/cpu/aarch64.py

@@ -2837,9 +2837,7 @@ def DMB(cpu, bar_imm_op):
         insn_rx += "1{5}"
 
         assert re.match(insn_rx, cpu.insn_bit_str)
-
         # XXX: Assumes sequential execution.
-        pass
 
     # XXX: Support DUP (element).
     @instruction

manticore/native/cpu/arm.py

@@ -4,6 +4,7 @@
 import struct
 
 import capstone as cs
+import operator as ops
 
 from .abstractcpu import Abi, Cpu, Interruption, Operand, RegisterFile, SyscallAbi
 from .abstractcpu import instruction as abstract_instruction
@@ -763,8 +764,8 @@ def canonicalize_instruction_name(instr):
                 return "ASR"
         return OP_NAME_MAP.get(name, name)
 
-    def _wrap_operands(self, ops):
-        return [Armv7Operand(self, op) for op in ops]
+    def _wrap_operands(self, operands):
+        return [Armv7Operand(self, op) for op in operands]
 
     def should_commit_flags(cpu):
         # workaround for a capstone bug (issue #980);
@@ -1515,7 +1516,8 @@ def _bitwise_instruction(cpu, operation, dest, op1, op2=None):
             op2_val, carry = op2.read(with_carry=True)
             result = operation(op1.read(), op2_val)
         else:
-            op1_val, carry = op1.read(with_carry=True)
+            # We _do_ use this form, contrary to what LGTM says
+            op1_val, carry = op1.read(with_carry=True)  # lgtm [py/call/wrong-arguments]
             result = operation(op1_val)
         if dest is not None:
             dest.write(result)
@@ -1524,9 +1526,9 @@ def _bitwise_instruction(cpu, operation, dest, op1, op2=None):
     @instruction(can_take_denormalized_mod_imm=True)
     def ORR(cpu, dest, op1, op2=None):
         if op2 is not None:
-            cpu._bitwise_instruction(lambda x, y: x | y, dest, op1, op2)
+            cpu._bitwise_instruction(ops.or_, dest, op1, op2)
         else:
-            cpu._bitwise_instruction(lambda x, y: x | y, dest, dest, op1)
+            cpu._bitwise_instruction(ops.or_, dest, dest, op1)
 
     @instruction(can_take_denormalized_mod_imm=True)
     def ORN(cpu, dest, op1, op2=None):
@@ -1538,20 +1540,20 @@ def ORN(cpu, dest, op1, op2=None):
     @instruction(can_take_denormalized_mod_imm=True)
     def EOR(cpu, dest, op1, op2=None):
         if op2 is not None:
-            cpu._bitwise_instruction(lambda x, y: x ^ y, dest, op1, op2)
+            cpu._bitwise_instruction(ops.xor, dest, op1, op2)
         else:
-            cpu._bitwise_instruction(lambda x, y: x ^ y, dest, dest, op1)
+            cpu._bitwise_instruction(ops.xor, dest, dest, op1)
 
     @instruction(can_take_denormalized_mod_imm=True)
     def AND(cpu, dest, op1, op2=None):
         if op2 is not None:
-            cpu._bitwise_instruction(lambda x, y: x & y, dest, op1, op2)
+            cpu._bitwise_instruction(ops.and_, dest, op1, op2)
         else:
-            cpu._bitwise_instruction(lambda x, y: x & y, dest, dest, op1)
+            cpu._bitwise_instruction(ops.and_, dest, dest, op1)
 
     @instruction(can_take_denormalized_mod_imm=True)
     def TEQ(cpu, op1, op2=None):
-        cpu._bitwise_instruction(lambda x, y: x ^ y, None, op1, op2)
+        cpu._bitwise_instruction(ops.xor, None, op1, op2)
         cpu.commit_flags()
 
     @instruction(can_take_denormalized_mod_imm=True)

manticore/native/memory.py

@@ -63,6 +63,7 @@ def __init__(self, mem, address, size, message=None, policy="MINMAX"):
             self.message = f"Concretizing memory address {address} size {size}"
         else:
             self.message = message
+        super().__init__(self.message, address)
         self.mem = mem
         self.address = address
         self.size = size

manticore/platforms/decree.py

@@ -1082,12 +1082,8 @@ def sys_random(self, cpu, buf, count, rnd_bytes):
 
         data = []
         for i in range(count):
-            if False:
-                # Too slow for the new age.
-                value = self.constraints.new_bitvec(8, name="RANDOM_%04d" % self.random)
-                self.constraints.add(symb == cgcrandom.stream[self.random])
-            else:
-                value = cgcrandom.stream[self.random]
+            # TODO - should value be symbolic?
+            value = cgcrandom.stream[self.random]
             data.append(value)
             self.random += 1
 

manticore/platforms/linux.py

@@ -161,8 +161,11 @@ def sync(self) -> None:
         return
 
 
-class ProcSelfMaps(File):
+# TODO - we should consider refactoring File so that we don't have to mute these errors
+class ProcSelfMaps(File):  # lgtm [py/missing-call-to-init]
     def __init__(self, flags: int, linux):
+        # WARN: Does not call File.__init__. Should have the File API, but we manually
+        # manage the underlying file and mode
         self.file = tempfile.NamedTemporaryFile(mode="w", delete=False)
         self.file.write(linux.current.memory.__proc_self__)
         self.file.close()
@@ -172,8 +175,10 @@ def __init__(self, flags: int, linux):
         self.file = open(self.file.name, mode)
 
 
-class Directory(File):
+class Directory(File):  # lgtm [py/missing-call-to-init]
     def __init__(self, path: str, flags: int):
+        # WARN: Does not call File.__init__ because we don't want to open the directory,
+        # even though we still want it to present the same API as File
         assert os.path.isdir(path)
 
         self.fd = os.open(path, flags)

manticore/wasm/executor.py

@@ -1454,8 +1454,8 @@ def f32_demote_f64(self, store, stack):
             stack.push(F32.cast(c1))
             return
         raise NotImplementedError("f32_demote_f64")
-        c1 = struct.unpack("f", struct.pack("d", c1)[:4])[0]
-        stack.push(F32.cast(c1))
+        # c1 = struct.unpack("f", struct.pack("d", c1)[:4])[0]
+        # stack.push(F32.cast(c1))
 
     def f64_convert_s_i32(self, store, stack):
         stack.has_type_on_top(I32, 1)

manticore/wasm/structure.py

@@ -1082,7 +1082,7 @@ def _invoke_inner(self, stack: "Stack", funcaddr: FuncAddr, store: Store):
         ty = f.type
         assert len(ty.result_types) <= 1
         local_vars: typing.List[Value] = []
-        for v in [stack.pop() for _t in ty.param_types][::-1]:
+        for v in [stack.pop() for _ in ty.param_types][::-1]:
             assert not isinstance(v, (Label, Activation))
             local_vars.append(v)
 
@@ -1846,7 +1846,7 @@ def get_frame(self) -> Activation:
         raise RuntimeError("Couldn't find a frame on the stack")
 
 
-class AtomicStack(Stack):
+class AtomicStack(Stack):  # lgtm [py/missing-call-to-init]
     """
     Allows for the rolling-back of the stack in the event of a concretization exception.
     Inherits from Stack so that the types will be correct, but never calls `super`.

scripts/gdb.py

@@ -95,7 +95,6 @@ def getM(m):
         return int(correspond(f"x/xg {m}\n").strip().split("\t")[-1], 0)
     except Exception as e:
         raise e
-        return 0
 
 
 def getPid():
@@ -138,5 +137,5 @@ def get_arch():
         _arch = "armv7"
     else:
         print(infotarget)
-        raise NotImplemented
+        raise NotImplementedError
     return _arch

scripts/prof.py

@@ -7,15 +7,19 @@
 from sys import argv, exit
 
 from manticore.native import Manticore
+from manticore.core.plugin import Profiler
 
 
 def profile(program, sort="cumulative"):
     print(f'[*] Profiling program "{program}"')
 
     m = Manticore(program)
-    m.run(should_profile=True)
+    profiler = Profiler()
+    m.register_plugin(profiler)
+    m.run()
+    m.finalize()
 
-    stats = m.get_profiling_stats()
+    stats = profiler.get_profiling_data()
     print(f"[*] Loaded profiling data.")
 
     if stats is None:

tests/auto_generators/make_dump.py

@@ -158,7 +158,7 @@ def read_operand(o):
     elif o.type == X86_OP_REG:
         reg_name = str(instruction.reg_name(o.reg).upper())
         return gdb.getR(reg_name)
-    raise NotImplemented
+    raise NotImplementedError(f"Unknown operand typ: {o.type}")
 
 
 STACK_INSTRUCTIONS = [