From dd066c828c647a6a393658e211e7df0df39dbcd3 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 23 Jun 2015 15:28:42 -0200 Subject: [PATCH] [Checkers] checker for printf format strings. --- infer/src/checkers/printfArgs.ml | 222 ++++++++++++++++++ infer/src/checkers/printfArgs.mli | 16 ++ infer/src/checkers/registerCheckers.ml | 1 + .../java/checkers/PrintfArgsChecker.java | 67 ++++++ .../java/checkers/PrintfArgsCheckerTest.java | 53 +++++ 5 files changed, 359 insertions(+) create mode 100644 infer/src/checkers/printfArgs.ml create mode 100644 infer/src/checkers/printfArgs.mli create mode 100644 infer/tests/codetoanalyze/java/checkers/PrintfArgsChecker.java create mode 100644 infer/tests/endtoend/java/checkers/PrintfArgsCheckerTest.java diff --git a/infer/src/checkers/printfArgs.ml b/infer/src/checkers/printfArgs.ml new file mode 100644 index 000000000..55c4ef860 --- /dev/null +++ b/infer/src/checkers/printfArgs.ml @@ -0,0 +1,222 @@ +(* +* Copyright (c) 2013 - Facebook. All rights reserved. +*) + +module L = Logging +module F = Format +open Utils + +type printf_signature = { + unique_id: string; + format_pos: int; + fixed_pos: int list; + vararg_pos: int option +} + +let printf_signature_to_string + (printf: printf_signature): string = + Printf.sprintf + "{%s; %d [%s] %s}" + printf.unique_id + printf.format_pos + (String.concat "," (list_map string_of_int printf.fixed_pos)) + (match printf.vararg_pos with | Some i -> string_of_int i | _ -> "-") + +let printf_like_functions = + ref + [ + { unique_id = "java.io.PrintStream.printf(java.lang.String,java.lang.Object[]):java.io.PrintStream"; + format_pos = 1; + fixed_pos = []; + vararg_pos = Some 2 }; + { unique_id = "java.io.PrintStream.printf(java.lang.Locale,java.lang.String,java.lang.Object[]):java.io.PrintStream"; + format_pos = 2; + fixed_pos = []; + vararg_pos = Some 3 }; + { unique_id = "java.lang.String(java.lang.String,java.lang.Object[]):java.lang.String"; + format_pos = 1; + fixed_pos = []; + vararg_pos = Some 2 }; + { unique_id = "java.lang.String(java.lang.Locale,java.lang.String,java.lang.Object[]):java.lang.String"; + format_pos = 2; + fixed_pos = []; + vararg_pos = Some 3 }; + ] + +let add_printf_like_function plf = + printf_like_functions := plf :: !printf_like_functions + + +let printf_like_function + (proc_name: Procname.t): printf_signature option = + try + Some ( + list_find + (fun printf -> string_equal printf.unique_id (Procname.to_unique_id proc_name)) + !printf_like_functions) + with Not_found -> None + +let default_format_type_name + (format_type: string): string = + match format_type with + | "d" | "i" | "u" | "x" | "X" | "o" -> "java.lang.Integer" + | "a" | "A" | "f" | "F" | "g" | "G" | "e" | "E" -> "java.lang.Double" + | "c" -> "java.lang.Character" + | "b" -> "java.lang.Boolean" + | "s" -> "java.lang.String" + | "h" | "H" -> "java.lang.Object" + | _ -> "unknown" + +let format_type_matches_given_type + (format_type: string) + (given_type: string): bool = + match format_type with + | "d" | "i" | "u" | "x" | "X" | "o" -> + list_mem + string_equal + given_type + ["java.lang.Integer"; "java.lang.Long"; "java.lang.Short"; "java.lang.Byte"] + | "a" | "A" | "f" | "F" | "g" | "G" | "e" | "E" -> + list_mem + string_equal + given_type + ["java.lang.Double"; "java.lang.Float"] + | "c" -> string_equal given_type "java.lang.Character" + | "b" | "h" | "H" | "s" -> true (* accepts pretty much anything, even null *) + | _ -> false + +(* The format string and the nvar for the fixed arguments and the nvar of the varargs array *) +let format_arguments + (printf: printf_signature) + (args: (Sil.exp * Sil.typ) list): (string option * (Sil.exp list) * (Sil.exp option)) = + + let format_string = match list_nth args printf.format_pos with + | Sil.Const (Sil.Cstr fmt), _ -> Some fmt + | _ -> None in + + let fixed_nvars = list_map + (fun i -> fst (list_nth args i)) + printf.fixed_pos in + let varargs_nvar = match printf.vararg_pos with + | Some pos -> Some (fst (list_nth args pos)) + | None -> None in + + format_string, fixed_nvars, varargs_nvar + +(* Extract type names from format string *) +let rec format_string_type_names + (fmt_string: string) + (start: int): string list = + try + let fmt_re = Str.regexp "%[0-9]*\\.?[0-9]*[A-mo-z]" in (* matches '%2.1d' etc. *) + let _ = Str.search_forward fmt_re fmt_string start in + let fmt_match = Str.matched_string fmt_string in + let fmt_type = String.sub fmt_match ((String.length fmt_match) - 1) 1 in + fmt_type:: format_string_type_names fmt_string (Str.match_end ()) + with Not_found -> [] + +let printf_args_name = "CHECKERS_PRINTF_ARGS" + +let callback_printf_args + (all_procs : Procname.t list) + (get_proc_desc: Procname.t -> Cfg.Procdesc.t option) + (idenv: Idenv.t) + (tenv: Sil.tenv) + (proc_name: Procname.t) + (proc_desc : Cfg.Procdesc.t) : unit = + + (* Check if format string lines up with arguments *) + let rec check_type_names instr_loc n_arg instr_proc_name fmt_type_names arg_type_names = + let instr_name = Procname.to_simplified_string instr_proc_name in + let instr_line = Sil.loc_to_string instr_loc in + match (fmt_type_names, arg_type_names) with + | ft:: fs, gt:: gs -> + if not (format_type_matches_given_type ft gt) then + let description = Printf.sprintf + "%s at line %s: parameter %d is expected to be of type %s but %s was given." + instr_name + instr_line + n_arg + (default_format_type_name ft) + gt in + Checkers.ST.report_error + proc_name + proc_desc + printf_args_name + instr_loc + description + else + check_type_names instr_loc (n_arg + 1) instr_proc_name fs gs + | [], [] -> () + | _ -> + let description = Printf.sprintf + "format string arguments don't mach provided arguments in %s at line %s" + instr_name + instr_line in + Checkers.ST.report_error + proc_name + proc_desc + printf_args_name + instr_loc + description in + + (* Get the array ivar for a given nvar *) + let rec array_ivar instrs nvar = + match instrs, nvar with + | Sil.Letderef (id, Sil.Lvar iv, _, _):: _, Sil.Var nid + when Ident.equal id nid -> iv + | i:: is, _ -> array_ivar is nvar + | _ -> raise Not_found in + + let rec fixed_nvar_type_name instrs nvar = + match nvar with + | Sil.Var nid -> ( + match instrs with + | Sil.Letderef (id, Sil.Lvar iv, t, _):: _ + when Ident.equal id nid -> PatternMatch.get_type_name t + | i:: is -> fixed_nvar_type_name is nvar + | _ -> raise Not_found) + | Sil.Const c -> PatternMatch.java_get_const_type_name c + | _ -> raise (Failure "Could not resolve fixed type name") in + + let do_instr + (node: Cfg.Node.t) + (instr: Sil.instr): unit = + match instr with + | Sil.Call (_, Sil.Const (Sil.Cfun pn), args, cl, _) -> ( + match printf_like_function pn with + | Some printf -> ( + try + let fmt, fixed_nvars, array_nvar = format_arguments printf args in + let instrs = Cfg.Node.get_instrs node in + let fixed_nvar_type_names = list_map (fixed_nvar_type_name instrs) fixed_nvars in + let vararg_ivar_type_names = match array_nvar with + | Some nvar -> ( + let ivar = array_ivar instrs nvar in + PatternMatch.get_vararg_type_names node ivar) + | None -> [] in + match fmt with + | Some fmt -> + check_type_names + cl + (printf.format_pos + 1) + pn + (format_string_type_names fmt 0) + (fixed_nvar_type_names@ vararg_ivar_type_names) + | None -> + Checkers.ST.report_error + proc_name + proc_desc + printf_args_name + cl + "Format string must be string literal" + with e -> + L.stderr + "%s Exception when analyzing %s: %s@." + printf_args_name + (Procname.to_string proc_name) + (Printexc.to_string e)) + | None -> ()) + | _ -> () in + + Cfg.Procdesc.iter_instrs do_instr proc_desc diff --git a/infer/src/checkers/printfArgs.mli b/infer/src/checkers/printfArgs.mli new file mode 100644 index 000000000..a504d98b6 --- /dev/null +++ b/infer/src/checkers/printfArgs.mli @@ -0,0 +1,16 @@ +(* +* Copyright (c) 2013 - Facebook. All rights reserved. +*) + + +type printf_signature = { + unique_id: string; + format_pos: int; + fixed_pos: int list; + vararg_pos: int option +} + +val add_printf_like_function : printf_signature -> unit + + +val callback_printf_args: Callbacks.proc_callback_t diff --git a/infer/src/checkers/registerCheckers.ml b/infer/src/checkers/registerCheckers.ml index 6e0683bf1..3af2c5c2e 100644 --- a/infer/src/checkers/registerCheckers.ml +++ b/infer/src/checkers/registerCheckers.ml @@ -29,6 +29,7 @@ let active_procedure_checkers () = Checkers.callback_check_field_access, false; ImmutableChecker.callback_check_immutable_cast, checkers_enabled; RepeatedCallsChecker.callback_check_repeated_calls, checkers_enabled; + PrintfArgs.callback_printf_args, checkers_enabled; ] in list_map (fun (x, y) -> (x, y, Some Sil.Java)) l in let c_cpp_checkers = diff --git a/infer/tests/codetoanalyze/java/checkers/PrintfArgsChecker.java b/infer/tests/codetoanalyze/java/checkers/PrintfArgsChecker.java new file mode 100644 index 000000000..02c15af6a --- /dev/null +++ b/infer/tests/codetoanalyze/java/checkers/PrintfArgsChecker.java @@ -0,0 +1,67 @@ +/* +* Copyright (c) 2013- Facebook. +* All rights reserved. +*/ + +package codetoanalyze.java.checkers; + + +import java.io.PrintStream; + +import android.annotation.SuppressLint; + + +public class PrintfArgsChecker { + + void argumentsMatch(PrintStream out) { + out.printf("Hello %s", "world"); + } + + @SuppressLint("CHECKERS_PRINTF_ARGS") + void suppressed(PrintStream out) { + out.printf("Hello %d", "world"); + } + + @SuppressLint("checkers-printf-args") + void normalizedSuppressed(PrintStream out) { + out.printf("Hello %d", "world"); + } + + @SuppressLint("OTHER_CHECKER") + void notSuppressed(PrintStream out) { + out.printf("Hello %d", "world"); + } + + void stringInsteadOfInteger(PrintStream out) { + out.printf("Hello %d", "world"); + } + + void wrongNumberOfArguments(PrintStream out) { + out.printf("Hello %d, World %s", 10, "string", 1.5); + } + + Integer field; + + void fieldAccess(PrintStream out) { + out.printf("%d %s%n", field, field.toString()); + } + + void stringConcat(PrintStream out) { + out.printf("%s" + "%s", "hello", "world"); + } + + void formatStringIsNotLiteral(PrintStream out) { + String format = "%s %s"; + out.printf(format, "hello", "world"); + } + +} + +@SuppressLint("checkers-printf-args") +class SuppressedPrintfArgsChecker { + + void classSuppressed(PrintStream out) { + out.printf("Hello %d", "world"); + } + +} diff --git a/infer/tests/endtoend/java/checkers/PrintfArgsCheckerTest.java b/infer/tests/endtoend/java/checkers/PrintfArgsCheckerTest.java new file mode 100644 index 000000000..c3fd38e0e --- /dev/null +++ b/infer/tests/endtoend/java/checkers/PrintfArgsCheckerTest.java @@ -0,0 +1,53 @@ +/* +* Copyright (c) 2013- Facebook. +* All rights reserved. +*/ + +package endtoend.java.checkers; + +import static org.hamcrest.MatcherAssert.assertThat; +import static utils.matchers.ResultContainsExactly.containsExactly; + +import org.junit.BeforeClass; +import org.junit.Test; + +import java.io.IOException; + +import utils.InferException; +import utils.InferResults; + +public class PrintfArgsCheckerTest { + + + public static final String SOURCE_FILE = + "infer/tests/codetoanalyze/java/checkers/PrintfArgsChecker.java"; + + public static final String CHECKERS_PRINTF_ARGS = "CHECKERS_PRINTF_ARGS"; + + private static InferResults inferResults; + + @BeforeClass + public static void loadResults() throws InterruptedException, IOException { + inferResults = + InferResults.loadCheckersResults(PrintfArgsCheckerTest.class, SOURCE_FILE); + } + + @Test + public void matchErrors() + throws IOException, InterruptedException, InferException { + String[] methods = { + "notSuppressed", + "stringInsteadOfInteger", + "wrongNumberOfArguments", + "formatStringIsNotLiteral", + }; + assertThat( + "Results should contain " + CHECKERS_PRINTF_ARGS, + inferResults, + containsExactly( + CHECKERS_PRINTF_ARGS, + SOURCE_FILE, + methods)); + } + +}