From 3afbcaa02e6bfd1ebf3ef66a2c08d425e35d0ba9 Mon Sep 17 00:00:00 2001 From: Ricardo Baratto Date: Mon, 2 Dec 2024 09:47:32 -0500 Subject: [PATCH 1/4] REP: allow patch candidate targets to be unspecified in that case we will include all targets, instead of none --- chb/cmdline/chkx | 5 +++-- chb/cmdline/reportcmds.py | 12 ++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/chb/cmdline/chkx b/chb/cmdline/chkx index 7ee9209a..aaed6597 100755 --- a/chb/cmdline/chkx +++ b/chb/cmdline/chkx @@ -1184,8 +1184,9 @@ def parse() -> argparse.Namespace: report_patchcandidates.add_argument( "--targets", nargs="*", - default=[], - help="list of target library functions to include") + default=['all'], + help="list of target library functions to include. If not passed, all " + "library functions found will be included.") report_patchcandidates.add_argument( "--verbose", "-v", action="store_true", diff --git a/chb/cmdline/reportcmds.py b/chb/cmdline/reportcmds.py index 610df7cd..a994db85 100644 --- a/chb/cmdline/reportcmds.py +++ b/chb/cmdline/reportcmds.py @@ -66,7 +66,8 @@ from chb.util.loggingutil import chklogger if TYPE_CHECKING: - from chb.api.CallTarget import StubTarget + from chb.api.CallTarget import ( + StubTarget, CallTarget) from chb.api.FunctionStub import SOFunction from chb.app.AppAccess import AppAccess from chb.app.BasicBlock import BasicBlock @@ -1172,6 +1173,13 @@ def find_spare_instruction( n_calls: int = 0 libcalls = LibraryCallCallsites() + include_all = xtargets == ['all'] + + def include_target(target: 'CallTarget') -> bool: + if include_all: + return True + return target.name in xtargets + for (faddr, blocks) in app.call_instructions().items(): fn = app.function(faddr) @@ -1179,7 +1187,7 @@ def find_spare_instruction( for instr in instrs: n_calls += 1 calltgt = instr.call_target - if calltgt.is_so_target and calltgt.name in xtargets: + if calltgt.is_so_target and include_target(calltgt): libcalls.add_library_callsite(faddr, baddr, instr) print("Number of calls: " + str(n_calls)) From ca959b24d885f23d802cd79de1a183a68fc5ab7b Mon Sep 17 00:00:00 2001 From: Ricardo Baratto Date: Mon, 2 Dec 2024 13:11:00 -0500 Subject: [PATCH 2/4] REP: add json output to chkx report patchcandidates Moved all the json functionality into the LibrarySideEffect class so that the json and txt output could share the same path. --- chb/buffer/LibraryCallCallsites.py | 65 ++++++++++++++ chb/cmdline/reportcmds.py | 135 ++++++++++++++--------------- 2 files changed, 129 insertions(+), 71 deletions(-) diff --git a/chb/buffer/LibraryCallCallsites.py b/chb/buffer/LibraryCallCallsites.py index 28e4efa6..3728b8c0 100644 --- a/chb/buffer/LibraryCallCallsites.py +++ b/chb/buffer/LibraryCallCallsites.py @@ -40,10 +40,13 @@ from chb.api.CallTarget import StubTarget from chb.api.FunctionStub import SOFunction from chb.app.Instruction import Instruction + from chb.app.AppAccess import AppAccess + from chb.app.Function import Function from chb.models.BTerm import BTerm, BTermArithmetic from chb.models.FunctionSummary import FunctionSummary from chb.models.FunctionPrecondition import ( FunctionPrecondition, PreDerefWrite) + from chb.mips.MIPSInstruction import MIPSInstruction class LibraryCallSideeffect: @@ -233,6 +236,68 @@ def lentype(self) -> str: else: return lenterm + @property + def dstoffset(self) -> Optional[int]: + """Returns the stack offset for our destination buffer""" + if self.dstarg is None: + return None + return self.dstarg.stack_address_offset() + + def find_spare_instruction(self, + app: 'AppAccess', + fn: 'Function', + ) -> Optional[str]: + if app.is_mips: + block = fn.block(self.baddr) + found = None + for (addr, instr) in block.instructions.items(): + instr = cast("MIPSInstruction", instr) + if instr.iaddr > self.instr.iaddr: + break + if instr.is_load_instruction: + if str(instr.operands[0]) == "t9": + found = instr.iaddr + return found + return None + + def to_json_result(self, app: 'AppAccess') -> JSONResult: + if self.dstarg is None: + chklogger.logger.warning( + "No expression found for destination argument: %s", self) + return JSONResult("librarycallsideeffect", {}, "fail", + "No expression found for destination argument") + dstoffset = self.dstoffset + if dstoffset is None: + chklogger.logger.warning( + "No value found for destination stack offset : %s", self) + return JSONResult("librarycallsideeffect", {}, "fail", + "No value found for destination stack offset") + + fn = app.function(self.faddr) + stackframe = fn.stackframe + stackbuffer = stackframe.get_stack_buffer(dstoffset) + if stackbuffer is None: + chklogger.logger.warning( + "No stackbuffer found for %s at offset %s", self.instr, dstoffset) + return JSONResult("librarycallsideeffect", {}, "fail", + "No stackbuffer found for %s at offset %s" % + (self.instr, dstoffset)) + + buffersize = stackbuffer.size + spare = self.find_spare_instruction(app, fn) + + content: Dict[str, Any] = {} + content["annotation"] = self.instr.annotation + content["faddr"] = self.faddr + content["iaddr"] = self.instr.iaddr + content["buffersize"] = buffersize + content["target-function"] = self.summary.name + content["stack-offset"] = dstoffset + content["length-argument"] = str(self.lenarg) + content["spare"] = spare + + return JSONResult("librarycallsideeffect", content, "ok") + def __str__(self) -> str: return self.instr.iaddr + ": " + str(self.instr.annotation) diff --git a/chb/cmdline/reportcmds.py b/chb/cmdline/reportcmds.py index a994db85..1d9cee4e 100644 --- a/chb/cmdline/reportcmds.py +++ b/chb/cmdline/reportcmds.py @@ -991,6 +991,30 @@ def perc(v: float) -> str: exit(0) +def write_json_result(xfilename: Optional[str], + jresult: JSONResult, + schema_name: str, + ) -> None: + if jresult.is_ok: + okresult = JU.jsonok(schema_name, jresult.content) + if xfilename is not None: + filename = xfilename + ".json" + with open(filename, "w") as fp: + json.dump(okresult, fp, indent=2) + chklogger.logger.info("JSON output written to " + filename) + else: + print(json.dumps(okresult)) + else: + failresult = JU.jsonfail(jresult.reason) + if xfilename is not None: + filename = xfilename + ".json" + with open(filename, "w") as fp: + json.dump(failresult, fp, indent=2) + chklogger.logger.warning( + "JSON failure output written to " + filename) + else: + print(json.dumps(failresult)) + def report_buffer_bounds(args: argparse.Namespace) -> NoReturn: # arguments @@ -1024,49 +1048,27 @@ def report_buffer_bounds(args: argparse.Namespace) -> NoReturn: if calltgt.is_so_target: libcalls.add_library_callsite(faddr, baddr, instr) - def write_json_result( - xfilename: Optional[str], jresult: JSONResult) -> None: - if jresult.is_ok: - okresult = JU.jsonok("bufferboundsassessment", jresult.content) - if xfilename is not None: - filename = xfilename + ".json" - with open(filename, "w") as fp: - json.dump(okresult, fp, indent=2) - chklogger.logger.info("JSON output written to " + filename) - else: - print(json.dumps(okresult)) - else: - failresult = JU.jsonfail(jresult.reason) - if xfilename is not None: - filename = xfilename + ".json" - with open(filename, "w") as fp: - json.dump(failresult, fp, indent=2) - chklogger.logger.warning( - "JSON failure output written to " + filename) - else: - print(json.dumps(failresult)) - if xjson: content: Dict[str, Any] = {} xinfodata = xinfo.to_json_result() if xinfodata.is_ok: content["identification"] = xinfodata.content else: - write_json_result(xoutput, xinfodata) + write_json_result(xoutput, xinfodata, "bufferboundsassessment") analysisstats = app.result_metrics.to_json_result() if analysisstats.is_ok: content["analysis"] = analysisstats.content else: - write_json_result(xoutput, analysisstats) + write_json_result(xoutput, analysisstats, "bufferboundsassessment") libcallsresult = libcalls.to_json_result() if libcallsresult.is_ok: content["bounds"] = libcallsresult.content else: - write_json_result(xoutput, libcallsresult) + write_json_result(xoutput, libcallsresult,"bufferboundsassessment" ) write_json_result(xoutput, JSONResult( - "libcboundsanalysis", content, "ok")) + "libcboundsanalysis", content, "ok"), "bufferboundsassessment") if not xverbose: exit(0) @@ -1154,20 +1156,6 @@ def report_patch_candidates(args: argparse.Namespace) -> NoReturn: xinfo = XI.XInfo() xinfo.load(path, xfile) - def find_spare_instruction( - block: "BasicBlock", iaddr: str) -> Optional[str]: - if xinfo.is_mips: - found = None - for (addr, instr) in block.instructions.items(): - instr = cast("MIPSInstruction", instr) - if instr.iaddr > iaddr: - break - if instr.is_load_instruction: - if str(instr.operands[0]) == "t9": - found = instr.iaddr - return found - return None - app = UC.get_app(path, xfile, xinfo) n_calls: int = 0 @@ -1181,8 +1169,6 @@ def include_target(target: 'CallTarget') -> bool: return target.name in xtargets for (faddr, blocks) in app.call_instructions().items(): - fn = app.function(faddr) - for (baddr, instrs) in blocks.items(): for instr in instrs: n_calls += 1 @@ -1190,41 +1176,48 @@ def include_target(target: 'CallTarget') -> bool: if calltgt.is_so_target and include_target(calltgt): libcalls.add_library_callsite(faddr, baddr, instr) - print("Number of calls: " + str(n_calls)) + chklogger.logger.debug("Number of calls: %s", n_calls) patchcallsites = libcalls.patch_callsites() + content: Dict[str, Any] = {} + if xjson: + xinfodata = xinfo.to_json_result() + if xinfodata.is_ok: + content["identification"] = xinfodata.content + else: + write_json_result(xoutput, xinfodata, "patchcandidates") + + patch_records = [] + for pc in sorted(patchcallsites, key=lambda pc:pc.faddr): - instr = pc.instr - dstarg = pc.dstarg - if dstarg is None: - chklogger.logger.warning( - "No expression found for destination argument: %s", - str(instr)) + jresult = pc.to_json_result(app) + if not jresult.is_ok: + chklogger.logger.warning("Couldn't process patch callsite %s", pc) continue - dstoffset = dstarg.stack_address_offset() - fn = app.function(pc.faddr) - stackframe = fn.stackframe - stackbuffer = stackframe.get_stack_buffer(dstoffset) - if stackbuffer is None: - chklogger.logger.warning( - "No stackbuffer found for %s at offset %s", - str(instr), str(dstoffset)) - continue - buffersize = stackbuffer.size - basicblock = fn.block(pc.baddr) - spare = find_spare_instruction(basicblock, instr.iaddr) - - print(" " + pc.instr.iaddr + " " + pc.instr.annotation) - print(" - faddr: " + pc.faddr) - print(" - iaddr: " + pc.instr.iaddr) - print(" - target function: " + str(pc.summary.name)) - print(" - stack offset: " + str(dstoffset)) - print(" - length argument: " + str(pc.lenarg)) - print(" - buffersize: " + str(buffersize)) - print(" - spare: " + str(spare)) + + patch_records.append(jresult.content) + + content["patch-records"] = patch_records + chklogger.logger.debug("Number of patch callsites: %s", len(content['patch-records'])) + + if xjson: + jcontent = JSONResult("patchcandidates", content, "ok") + write_json_result(xoutput, jcontent, "patchcandidates") + exit(0) + + for patch_record in content["patch-records"]: + print(" " + patch_record['iaddr'] + " " + patch_record['annotation']) + print(" - faddr: %s" % patch_record['faddr']) + print(" - iaddr: %s" % patch_record['iaddr']) + print(" - target function: %s" % patch_record['target-function']) + print(" - stack offset: %s" % patch_record['stack-offset']) + print(" - length argument: %s" % patch_record['length-argument']) + print(" - buffersize: %s" % patch_record['buffersize']) + print(" - spare: %s" % patch_record['spare']) print("") - print("Number of patch callsites: " + str(len(patchcallsites))) + print("Generated %d patch records from %d library calls" % + (len(content['patch-records']), n_calls)) exit(0) From e46f7c0b19fcbadc3cf16bb257ab2db2873c6be3 Mon Sep 17 00:00:00 2001 From: Ricardo Baratto Date: Mon, 2 Dec 2024 13:51:51 -0500 Subject: [PATCH 3/4] REP: If length argument is None, let that stay so json does the right thing We were converting it to a string with becomes 'None' which then throws off the json parser --- chb/buffer/LibraryCallCallsites.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/chb/buffer/LibraryCallCallsites.py b/chb/buffer/LibraryCallCallsites.py index 3728b8c0..885d17f7 100644 --- a/chb/buffer/LibraryCallCallsites.py +++ b/chb/buffer/LibraryCallCallsites.py @@ -293,7 +293,10 @@ def to_json_result(self, app: 'AppAccess') -> JSONResult: content["buffersize"] = buffersize content["target-function"] = self.summary.name content["stack-offset"] = dstoffset - content["length-argument"] = str(self.lenarg) + if self.lenarg is not None: + content["length-argument"] = str(self.lenarg) + else: + content["length-argument"] = None content["spare"] = spare return JSONResult("librarycallsideeffect", content, "ok") From ede8dc3eeeaedf10a555c4c6018af2cd796629ca Mon Sep 17 00:00:00 2001 From: Ricardo Baratto Date: Thu, 5 Dec 2024 20:27:36 -0500 Subject: [PATCH 4/4] REP: move the computation of the buffer size and spare instruction back to the reportcmds And pass them in to the to_json_result method. Instead of passing in an AppAccess object. Per Henny's code review, doing all that work inside of the LibraryCallSideEffect class creates undesirable dependencies. --- chb/buffer/LibraryCallCallsites.py | 53 +++--------------------------- chb/cmdline/reportcmds.py | 38 +++++++++++++++++++-- 2 files changed, 41 insertions(+), 50 deletions(-) diff --git a/chb/buffer/LibraryCallCallsites.py b/chb/buffer/LibraryCallCallsites.py index 885d17f7..afb2dbdd 100644 --- a/chb/buffer/LibraryCallCallsites.py +++ b/chb/buffer/LibraryCallCallsites.py @@ -40,13 +40,10 @@ from chb.api.CallTarget import StubTarget from chb.api.FunctionStub import SOFunction from chb.app.Instruction import Instruction - from chb.app.AppAccess import AppAccess - from chb.app.Function import Function from chb.models.BTerm import BTerm, BTermArithmetic from chb.models.FunctionSummary import FunctionSummary from chb.models.FunctionPrecondition import ( FunctionPrecondition, PreDerefWrite) - from chb.mips.MIPSInstruction import MIPSInstruction class LibraryCallSideeffect: @@ -236,56 +233,16 @@ def lentype(self) -> str: else: return lenterm - @property - def dstoffset(self) -> Optional[int]: - """Returns the stack offset for our destination buffer""" - if self.dstarg is None: - return None - return self.dstarg.stack_address_offset() - - def find_spare_instruction(self, - app: 'AppAccess', - fn: 'Function', - ) -> Optional[str]: - if app.is_mips: - block = fn.block(self.baddr) - found = None - for (addr, instr) in block.instructions.items(): - instr = cast("MIPSInstruction", instr) - if instr.iaddr > self.instr.iaddr: - break - if instr.is_load_instruction: - if str(instr.operands[0]) == "t9": - found = instr.iaddr - return found - return None - - def to_json_result(self, app: 'AppAccess') -> JSONResult: + def to_json_result(self, + dstoffset: int, + buffersize: int, + spare: Optional[str], + ) -> JSONResult: if self.dstarg is None: chklogger.logger.warning( "No expression found for destination argument: %s", self) return JSONResult("librarycallsideeffect", {}, "fail", "No expression found for destination argument") - dstoffset = self.dstoffset - if dstoffset is None: - chklogger.logger.warning( - "No value found for destination stack offset : %s", self) - return JSONResult("librarycallsideeffect", {}, "fail", - "No value found for destination stack offset") - - fn = app.function(self.faddr) - stackframe = fn.stackframe - stackbuffer = stackframe.get_stack_buffer(dstoffset) - if stackbuffer is None: - chklogger.logger.warning( - "No stackbuffer found for %s at offset %s", self.instr, dstoffset) - return JSONResult("librarycallsideeffect", {}, "fail", - "No stackbuffer found for %s at offset %s" % - (self.instr, dstoffset)) - - buffersize = stackbuffer.size - spare = self.find_spare_instruction(app, fn) - content: Dict[str, Any] = {} content["annotation"] = self.instr.annotation content["faddr"] = self.faddr diff --git a/chb/cmdline/reportcmds.py b/chb/cmdline/reportcmds.py index 1d9cee4e..87bb1dce 100644 --- a/chb/cmdline/reportcmds.py +++ b/chb/cmdline/reportcmds.py @@ -1065,7 +1065,7 @@ def report_buffer_bounds(args: argparse.Namespace) -> NoReturn: if libcallsresult.is_ok: content["bounds"] = libcallsresult.content else: - write_json_result(xoutput, libcallsresult,"bufferboundsassessment" ) + write_json_result(xoutput, libcallsresult, "bufferboundsassessment" ) write_json_result(xoutput, JSONResult( "libcboundsanalysis", content, "ok"), "bufferboundsassessment") @@ -1156,6 +1156,20 @@ def report_patch_candidates(args: argparse.Namespace) -> NoReturn: xinfo = XI.XInfo() xinfo.load(path, xfile) + def find_spare_instruction( + block: "BasicBlock", iaddr: str) -> Optional[str]: + if xinfo.is_mips: + found = None + for (addr, instr) in block.instructions.items(): + instr = cast("MIPSInstruction", instr) + if instr.iaddr > iaddr: + break + if instr.is_load_instruction: + if str(instr.operands[0]) == "t9": + found = instr.iaddr + return found + return None + app = UC.get_app(path, xfile, xinfo) n_calls: int = 0 @@ -1191,7 +1205,27 @@ def include_target(target: 'CallTarget') -> bool: patch_records = [] for pc in sorted(patchcallsites, key=lambda pc:pc.faddr): - jresult = pc.to_json_result(app) + instr = pc.instr + dstarg = pc.dstarg + if dstarg is None: + chklogger.logger.warning( + "No expression found for destination argument: %s", + str(instr)) + continue + dstoffset = dstarg.stack_address_offset() + fn = app.function(pc.faddr) + stackframe = fn.stackframe + stackbuffer = stackframe.get_stack_buffer(dstoffset) + if stackbuffer is None: + chklogger.logger.warning( + "No stackbuffer found for %s at offset %s", + str(instr), str(dstoffset)) + continue + buffersize = stackbuffer.size + basicblock = fn.block(pc.baddr) + spare = find_spare_instruction(basicblock, instr.iaddr) + + jresult = pc.to_json_result(dstoffset, buffersize, spare) if not jresult.is_ok: chklogger.logger.warning("Couldn't process patch callsite %s", pc) continue