Cwe248 #5
Cwe248 #5
Conversation
CHANGES.md
Outdated
| @@ -1,3 +1,9 @@ | |||
| 0.2-dev (2019-XX-XX) | |||
There was a problem hiding this comment.
Add your CWE as a new feature in 0.2-dev
| @@ -1,14 +1,15 @@ | |||
| # cwe_checker # | |||
src/cwe_checker.ml
Outdated
| @@ -6,116 +6,58 @@ open Yojson.Basic.Util | |||
|
|
|||
There was a problem hiding this comment.
merge this file as well please
src/checkers/cwe_248.ml
Outdated
|
|
||
|
|
||
| let check_cwe program project tid_map symbol_pairs = | ||
| (* Get all subfunctions *) |
There was a problem hiding this comment.
this comment is not needed.
src/checkers/cwe_248.ml
Outdated
| let check_cwe program project tid_map symbol_pairs = | ||
| (* Get all subfunctions *) | ||
| let subfunctions = Term.enum sub_t program in | ||
| (* collect all entry points of the program *) |
There was a problem hiding this comment.
or refactor the functionality ("get_all_cpp_entrypoints") to a new function.
src/checkers/cwe_248.ml
Outdated
| let subfunctions = Term.enum sub_t program in | ||
| (* collect all entry points of the program *) | ||
| let entry_points = Seq.filter subfunctions ~f:(fun subfn -> Term.has_attr subfn Sub.entry_point) in | ||
| (* TODO: The _start entry point calls a libc-function which then calls the main function. Since right now only direct |
There was a problem hiding this comment.
I think it would be better to move the TODOs out of the function, right above it. This would improve the readability.
src/checkers/cwe_248.ml
Outdated
| (* search for uncatched exceptions for each entry point, but accumulate the list of already checked functions. *) | ||
| (* TODO: Exceptions, that are catched when starting from one entry point, but not from another, are masked this | ||
| way. We should check whether this produces a lot of false negatives. *) | ||
| let _ = Seq.fold entry_points_with_main ~init:[] ~f:(fun already_checked_functions sub -> find_uncaught_exceptions ~tid_map:tid_map sub already_checked_functions program) in |
There was a problem hiding this comment.
is fold the right choice here? You do not accumulate results.
| | Some(tid) -> Some(Tid.name tid) | ||
| | None -> None | ||
|
|
||
| (* check whether block contains a direct call to a symbol with name symbol_name *) |
There was a problem hiding this comment.
we could refactor this function. Extract it as blk_contains_symbol and add it to utils/symbol_utils.ml
|
|
||
| (* Print the findings to the log *) | ||
| let print_uncatched_exception block_tid ~tid_map = | ||
| Log_utils.warn |
There was a problem hiding this comment.
FYI, you could use BAP native logging facilities. Just include Self() and you will get debug, info, warning, and error functions.
| val name : string | ||
| val version : string | ||
|
|
||
| val check_cwe : Bap.Std.program Bap.Std.term -> Bap.Std.project -> Bap.Std.word Bap.Std.Tid.Map.t -> string list list -> unit |
There was a problem hiding this comment.
the Bap.Std interface is designed to be opened (it tries not to pollute the namespace). So once it is open you can easily write the interface in a much more concise and readable way: program term -> project -> word Tid.Map.t -> string list list -> unit
Adding the check for CWE-248 into master branch.