From f37ed668886fac4be15f661e429f13e7f9d6c27c Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Mon, 16 May 2016 06:09:51 -0700 Subject: [PATCH] Added new taint source/sink for iOS Reviewed By: sblackshear Differential Revision: D3054009 fbshipit-source-id: 6f6a20a --- infer/models/objc/src/NSHTTPCookie.m | 22 ++++++ infer/models/objc/src/NSString.h | 6 +- infer/models/objc/src/NSString.m | 24 +++++++ infer/src/backend/tabulation.ml | 57 +++++++++++---- infer/src/backend/taint.ml | 58 +++++++++++++++ .../codetoanalyze/objc/errors/taint/sources.m | 44 ++++++++++++ infer/tests/endtoend/objc/Taint2Test.java | 72 +++++++++++++++++++ 7 files changed, 268 insertions(+), 15 deletions(-) create mode 100644 infer/models/objc/src/NSHTTPCookie.m create mode 100644 infer/tests/codetoanalyze/objc/errors/taint/sources.m create mode 100644 infer/tests/endtoend/objc/Taint2Test.java diff --git a/infer/models/objc/src/NSHTTPCookie.m b/infer/models/objc/src/NSHTTPCookie.m new file mode 100644 index 000000000..df0cb6fb0 --- /dev/null +++ b/infer/models/objc/src/NSHTTPCookie.m @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2015 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import +#import + +@interface NSHTTPCookie : NSObject +@property(readonly, copy) NSString* value; +@end + +@implementation NSHTTPCookie +@synthesize value; +- (NSString*)value { + return value; +} +@end diff --git a/infer/models/objc/src/NSString.h b/infer/models/objc/src/NSString.h index f4724e80f..f13603027 100644 --- a/infer/models/objc/src/NSString.h +++ b/infer/models/objc/src/NSString.h @@ -14,5 +14,9 @@ } + (instancetype)stringWithUTF8String:(const char*)bytes; - ++ (instancetype)stringWithString:(NSString*)aString; ++ (instancetype)stringWithFormat:(NSString*)format, ...; ++ (instancetype)localizedStringWithFormat:(NSString*)format, ...; +- (instancetype)initWithFormat:(NSString*)format, ...; +- (instancetype)initWithFormat:(NSString*)format arguments:(va_list)argList; @end diff --git a/infer/models/objc/src/NSString.m b/infer/models/objc/src/NSString.m index 309b9fafe..ad7706407 100644 --- a/infer/models/objc/src/NSString.m +++ b/infer/models/objc/src/NSString.m @@ -21,6 +21,20 @@ void __get_array_size(const UInt8); return s; } ++ (instancetype)stringWithString:(NSString*)aString { + NSString* s = [NSString alloc]; + s->value = aString->value; + return s; +} + ++ (instancetype)stringWithFormat:(NSString*)format, ... { + return format; +} + ++ (instancetype)localizedStringWithFormat:(NSString*)format, ... { + return format; +} + - (instancetype)initWithBytesNoCopy:(char*)bytes length:(NSUInteger)length encoding:(id)encoding @@ -33,4 +47,14 @@ void __get_array_size(const UInt8); } return self; } + +- (instancetype)initWithFormat:(NSString*)format arguments:(va_list)argList { + self->value = format->value; + return self; +} + +- (instancetype)initWithFormat:(NSString*)format, ... { + self->value = format->value; + return self; +} @end diff --git a/infer/src/backend/tabulation.ml b/infer/src/backend/tabulation.ml index 14df513cc..7ff9d63ea 100644 --- a/infer/src/backend/tabulation.ml +++ b/infer/src/backend/tabulation.ml @@ -805,7 +805,10 @@ let add_tainting_attribute att pvar_param prop = (* add tainting attributes to a list of paramenters *) let add_tainting_att_param_list prop param_nums formal_params att = - try + match param_nums with + | [n] when n<0 -> prop + | _ -> + try IList.map (fun n -> IList.nth formal_params n) param_nums |> IList.fold_left (fun prop param -> add_tainting_attribute att param prop) prop with Failure _ | Invalid_argument _ -> @@ -832,6 +835,41 @@ let mk_pre pre formal_params callee_pname callee_attrs = |> Prop.expose else pre +let report_taint_error e taint_info callee_pname caller_pname calling_prop = + let err_desc = + Errdesc.explain_tainted_value_reaching_sensitive_function + calling_prop + e + taint_info + callee_pname + (State.get_loc ()) in + let exn = + Exceptions.Tainted_value_reaching_sensitive_function + (err_desc, __POS__) in + Reporting.log_warning caller_pname exn + +let check_taint_on_variadic_function callee_pname caller_pname actual_params calling_prop = + let rec n_tail lst n = (* return the tail of a list from element n *) + if n = 1 then lst + else match lst with + | [] -> [] + | _::lst' -> n_tail lst' (n-1) in + let tainted_params = Taint.accepts_sensitive_params callee_pname None in + match tainted_params with + | [tp] when tp<0 -> + (* All actual params from abs(tp) should not be tainted. If we find one we give the warning *) + let tp_abs = abs tp in + L.d_strln ("Checking tainted actual parameters from parameter number "^ (string_of_int tp_abs) ^ " onwards."); + let actual_params' = n_tail actual_params tp_abs in + L.d_str "Paramters to be checked: [ "; + IList.iter(fun (e,_) -> + L.d_str (" " ^ (Sil.exp_to_string e) ^ " "); + match Prop.get_taint_attribute calling_prop e with + | Some (Sil.Ataint taint_info) -> report_taint_error e taint_info callee_pname caller_pname calling_prop + | _ -> ()) actual_params'; + L.d_strln" ]" + | _ -> () + (** Construct the actual precondition: add to the current state a copy of the (callee's) formal parameters instantiated with the actual parameters. *) @@ -919,7 +957,7 @@ let inconsistent_actualpre_missing actual_pre split_opt = (* perform the taint analysis check by comparing the taint atoms in [calling_pi] with the untaint atoms required by the [missing_pi] computed during abduction *) -let do_taint_check caller_pname callee_pname calling_prop missing_pi sub = +let do_taint_check caller_pname callee_pname calling_prop missing_pi sub actual_params = let calling_pi = Prop.get_pi calling_prop in (* get a version of [missing_pi] whose var names match the names in calling pi *) let missing_pi_sub = Prop.pi_sub sub missing_pi in @@ -948,17 +986,7 @@ let do_taint_check caller_pname callee_pname calling_prop missing_pi sub = let taint_info = match Prop.atom_get_exp_attribute taint_atom with | Some (_, Sil.Ataint taint_info) -> taint_info | _ -> failwith "Expected to get taint attr on atom" in - let err_desc = - Errdesc.explain_tainted_value_reaching_sensitive_function - calling_prop - e - taint_info - callee_pname - (State.get_loc ()) in - let exn = - Exceptions.Tainted_value_reaching_sensitive_function - (err_desc, __POS__) in - Reporting.log_warning caller_pname exn in + report_taint_error e taint_info callee_pname caller_pname calling_prop in IList.iter report_one_error taint_atoms in Sil.ExpMap.iter report_taint_errors taint_untaint_exp_map; (* filter out UNTAINT(e) atoms from [missing_pi] such that we have already reported a taint @@ -974,6 +1002,7 @@ let do_taint_check caller_pname callee_pname calling_prop missing_pi sub = (fun a -> Sil.atom_equal atom a) untaint_atoms) taint_untaint_exp_map) in + check_taint_on_variadic_function callee_pname caller_pname actual_params calling_prop; IList.filter not_untaint_atom missing_pi_sub let class_cast_exn pname_opt texp1 texp2 exp ml_loc = @@ -1026,7 +1055,7 @@ let exe_spec let do_split () = let missing_pi' = if Config.taint_analysis then - do_taint_check caller_pname callee_pname actual_pre missing_pi sub2 + do_taint_check caller_pname callee_pname actual_pre missing_pi sub2 actual_params else missing_pi in process_splitting actual_pre sub1 sub2 frame missing_pi' missing_sigma diff --git a/infer/src/backend/taint.ml b/infer/src/backend/taint.ml index b7410b102..faef0a193 100644 --- a/infer/src/backend/taint.ml +++ b/infer/src/backend/taint.ml @@ -44,6 +44,17 @@ let sources = [ taint_kind = Sil.Tk_shared_preferences_data; language = Config.Java }; + (* === iOS === *) + { + classname = "NSHTTPCookie"; + method_name = "value"; + ret_type = "NSString *"; + params = []; + is_static = false; + taint_kind = Sil.Tk_privacy_annotation; + language = Config.Clang + }; + ] @ FbTaint.sources (* list of (sensitive sinks, zero-indexed numbers of parameters that should not be tainted). note: @@ -162,6 +173,53 @@ let sinks = [ language = Config.Java; }, [0]); + (* === iOS === *) + ({ + classname = "NSString"; + method_name = "stringWithFormat:"; + ret_type = "instancetype"; + params = []; + is_static = true; + taint_kind = Sil.Tk_unknown; + language = Config.Clang; + }, [-2]); + ({ + classname = "NSString"; + method_name = "stringWithUTF8String:"; + ret_type = "instancetype"; + params = []; + is_static = true; + taint_kind = Sil.Tk_unknown; + language = Config.Clang + }, [-2]); + ({ + classname = "NSString"; + method_name = "localizedStringWithFormat:"; + ret_type = "instancetype"; + params = []; + is_static = true; + taint_kind = Sil.Tk_unknown; + language = Config.Clang + }, [-2]); + ({ + classname = "NSString"; + method_name = "initWithFormat:"; + ret_type = "instancetype"; + params = []; + is_static = false; + taint_kind = Sil.Tk_unknown; + language = Config.Clang + }, [-2]); + ({ + classname = "NSString"; + method_name = "stringWithString:"; + ret_type = "instancetype"; + params = []; + is_static = true; + taint_kind = Sil.Tk_unknown; + language = Config.Clang + }, [0]); + (* ==== iOS for testing only ==== *) ({ classname = "ExampleViewController"; diff --git a/infer/tests/codetoanalyze/objc/errors/taint/sources.m b/infer/tests/codetoanalyze/objc/errors/taint/sources.m new file mode 100644 index 000000000..e0f2b3d80 --- /dev/null +++ b/infer/tests/codetoanalyze/objc/errors/taint/sources.m @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2016 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import +#import +#import + +void testNSHTTPCookie1() { + + NSHTTPCookie* c = [NSHTTPCookie new]; + NSString* s = c.value; + + [NSString stringWithFormat:@"Test taint %@: ", s]; +} + +void testNSHTTPCookie2() { + + NSHTTPCookie* c = [NSHTTPCookie new]; + NSString* s = c.value; + + [NSString localizedStringWithFormat:@"Test taint %@: ", s]; +} + +void testNSHTTPCookie3() { + + NSHTTPCookie* c = [NSHTTPCookie new]; + NSString* s = c.value; + + [[NSString alloc] initWithFormat:@"Test taint %@", s]; +} + +void testNSHTTPCookie4() { + + NSHTTPCookie* c = [NSHTTPCookie new]; + NSString* s = c.value; + + [NSString stringWithString:s]; +} diff --git a/infer/tests/endtoend/objc/Taint2Test.java b/infer/tests/endtoend/objc/Taint2Test.java new file mode 100644 index 000000000..8b7bcc88a --- /dev/null +++ b/infer/tests/endtoend/objc/Taint2Test.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2013 - present Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package endtoend.objc; + +import static org.hamcrest.MatcherAssert.assertThat; +import static utils.matchers.ResultContainsExactly.containsExactly; + +import com.google.common.collect.ImmutableList; + +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +import java.io.IOException; + +import utils.DebuggableTemporaryFolder; +import utils.InferException; +import utils.InferResults; +import utils.InferRunner; + +public class Taint2Test { + + public static final String TaintFile = + "infer/tests/codetoanalyze/objc/errors/taint/sources.m"; + + public static final String TAINTED_VALUE = "TAINTED_VALUE_REACHING_SENSITIVE_FUNCTION"; + + private static ImmutableList inferCmd; + + @ClassRule + public static DebuggableTemporaryFolder folder = new DebuggableTemporaryFolder(); + + + @BeforeClass + public static void runInfer() throws InterruptedException, IOException { + inferCmd = InferRunner.createObjCInferCommandWithMLBuckets( + folder, + TaintFile, + "cf", + false); + } + + @Test + public void whenInferRunsOnTaintFileErrorFound() + throws InterruptedException, IOException, InferException { + InferResults inferResults = InferRunner.runInferObjC(inferCmd); + String[] methods = { + "testNSHTTPCookie1", + "testNSHTTPCookie2", + "testNSHTTPCookie3", + "testNSHTTPCookie4" + }; + + assertThat( + "Results should contain tainted value reaching sensitive function.", + inferResults, + containsExactly( + TAINTED_VALUE, + TaintFile, + methods + ) + ); + } + +}