diff --git a/analysis/test/lookupTest.ml b/analysis/test/lookupTest.ml index 4acdb2cfeb0..44aa8a048eb 100644 --- a/analysis/test/lookupTest.ml +++ b/analysis/test/lookupTest.ml @@ -533,7 +533,7 @@ let test_lookup_definitions context = "test.py:12:4-12:7 -> test.py:8:0-9:8"; "test.py:13:12-13:18 -> test.py:2:0-3:13"; "test.py:13:4-13:11 -> test.py:5:0-6:8"; - "test.py:2:16-2:19 -> builtins.pyi:76:0-93:34" ]; + "test.py:2:16-2:19 -> builtins.pyi:77:0-94:34" ]; assert_definition ~position:{ Location.line = 12; column = 0 } ~definition:None; assert_definition ~position:{ Location.line = 12; column = 4 } diff --git a/taint/model.ml b/taint/model.ml index 3bf414b981a..5eaf6eb6a10 100644 --- a/taint/model.ml +++ b/taint/model.ml @@ -560,6 +560,17 @@ let model_compatible ~type_parameters ~normalized_model_parameters = errors +let demangle_class_attribute name = + if String.is_substring ~substring:"__class__" name then + String.split name ~on:'.' + |> List.rev + |> function + | attribute :: "__class__" :: rest -> List.rev (attribute :: rest) |> String.concat ~sep:"." + | _ -> name + else + name + + let create ~resolution ?path ~configuration source = let global_resolution = Resolution.global_resolution resolution in let signatures = @@ -697,6 +708,7 @@ let create ~resolution ?path ~configuration source = let model_compatibility_errors = (* Make self as an explicit parameter in type's parameter list *) let implicit_to_explicit_self { Type.Callable.name; implicit_annotation } = + let name = demangle_class_attribute name in let open Type.Callable.RecordParameter in Named { name; annotation = implicit_annotation; default = false } in @@ -823,14 +835,22 @@ let get_global_model ~resolution ~expression = match Node.value expression, AccessPath.get_global ~resolution expression with | _, Some global -> Some global | Name (Name.Attribute { base; attribute; _ }), _ -> - let annotation = - match Resolution.resolve resolution base with - | Type.Optional annotation -> annotation - | annotation -> - if Type.is_meta annotation then - Type.single_parameter annotation - else - annotation + let is_meta, annotation = + let rec is_meta = function + | Type.Optional annotation -> is_meta annotation + | annotation -> + if Type.is_meta annotation then + true, Type.single_parameter annotation + else + false, annotation + in + is_meta (Resolution.resolve resolution base) + in + let attribute = + if is_meta then + Format.sprintf "__class__.%s" attribute + else + attribute in annotation |> Type.class_name diff --git a/taint/model.mli b/taint/model.mli index 20c26d212c3..2be5b155bd5 100644 --- a/taint/model.mli +++ b/taint/model.mli @@ -16,6 +16,9 @@ type t = { exception InvalidModel of string +(* Exposed for testing. *) +val demangle_class_attribute : string -> string + val get_callsite_model : call_target:[< Callable.t ] -> arguments:Expression.t Expression.Call.Argument.t list -> diff --git a/taint/test/integration/class_flows.py b/taint/test/integration/class_flows.py index 5a35b097997..59d0a5278b8 100644 --- a/taint/test/integration/class_flows.py +++ b/taint/test/integration/class_flows.py @@ -1,10 +1,11 @@ # @nolint -from typing import Type +from typing import Type, Optional class C: tainted_attribute: List[int] = [] + tainted_class_attribute: List[int] = [] not_tainted = 2 @@ -12,18 +13,39 @@ class D(C): pass -def tainted_attribute_flow(c: C) -> None: +def tainted_attribute_flow_issue(c: C) -> None: c.tainted_attribute = __test_source() -def untainted_flow(c: C) -> None: +def untainted_flow_not_issue(c: C) -> None: c.not_tainted = __test_source() -def tainted_attribute_for_class(c: Type[C]) -> None: +def tainted_attribute_for_class_not_issue(c: Type[C]) -> None: c.tainted_attribute = __test_source() -def tainted_attribute_through_inheritance(d: D) -> None: +def tainted_attribute_through_inheritance_not_issue(d: D) -> None: # TODO(T47337940): Support this. d.tainted_attribute = __test_source() + + +def tainted_class_attribute_through_instance_not_issue(c: C) -> None: + c.tainted_class_attribute = __test_source() + + +def tainted_class_attribute_through_class_issue(class_object: Type[C]) -> None: + class_object.tainted_class_attribute = __test_source() + + +def tainted_class_attribute_through_double_underscore_class_issue(c: C) -> None: + c.__class__.tainted_class_attribute = __test_source() + + +def tainted_class_attribute_through_optional_class_issue(class_object: Optional[Type[C]]) -> None: + if class_object is not None: + class_object.tainted_class_attribute = __test_source() + + +def global_class_attribute_issue() -> None: + C.tainted_class_attribute = __test_source() diff --git a/taint/test/integration/class_flows.py.cg b/taint/test/integration/class_flows.py.cg index 1fef7deb551..842ab9be9ae 100644 --- a/taint/test/integration/class_flows.py.cg +++ b/taint/test/integration/class_flows.py.cg @@ -1,6 +1,11 @@ @generated Call dependencies -class_flows.tainted_attribute_flow (fun) -> [__test_source (fun)] -class_flows.tainted_attribute_for_class (fun) -> [__test_source (fun)] -class_flows.tainted_attribute_through_inheritance (fun) -> [__test_source (fun)] -class_flows.untainted_flow (fun) -> [__test_source (fun)] +class_flows.global_class_attribute_issue (fun) -> [__test_source (fun)] +class_flows.tainted_attribute_flow_issue (fun) -> [__test_source (fun)] +class_flows.tainted_attribute_for_class_not_issue (fun) -> [__test_source (fun)] +class_flows.tainted_attribute_through_inheritance_not_issue (fun) -> [__test_source (fun)] +class_flows.tainted_class_attribute_through_class_issue (fun) -> [__test_source (fun)] +class_flows.tainted_class_attribute_through_double_underscore_class_issue (fun) -> [__test_source (fun) object::__class__ (method)] +class_flows.tainted_class_attribute_through_instance_not_issue (fun) -> [__test_source (fun)] +class_flows.tainted_class_attribute_through_optional_class_issue (fun) -> [__test_source (fun)] +class_flows.untainted_flow_not_issue (fun) -> [__test_source (fun)] diff --git a/taint/test/integration/class_flows.py.models b/taint/test/integration/class_flows.py.models index 19463d44b65..ed26dfba5f9 100644 --- a/taint/test/integration/class_flows.py.models +++ b/taint/test/integration/class_flows.py.models @@ -2,12 +2,13 @@ { "kind": "issue", "data": { - "callable": "class_flows.tainted_attribute_flow", - "callable_line": 15, + "callable": + "class_flows.tainted_class_attribute_through_double_underscore_class_issue", + "callable_line": 41, "code": 5002, - "line": 16, + "line": 42, "start": 4, - "end": 23, + "end": 39, "filename": "class_flows.py", "message": "Test flow. Data from [Test] source(s) may reach [Test] sink(s)", @@ -18,9 +19,9 @@ { "root": { "filename": "class_flows.py", - "line": 16, - "start": 39, - "end": 41 + "line": 42, + "start": 55, + "end": 57 }, "leaves": [ { "kind": "Test", "name": "__test_source" } ], "features": [ { "via": "special_source" } ] @@ -33,12 +34,116 @@ { "root": { "filename": "class_flows.py", - "line": 16, + "line": 42, "start": 4, - "end": 23 + "end": 39 }, "leaves": [ - { "kind": "Test", "name": "class_flows.C.tainted_attribute" } + { + "kind": "Test", + "name": "class_flows.C.__class__.tainted_class_attribute" + } + ] + } + ] + } + ] + } +} +{ + "kind": "issue", + "data": { + "callable": + "class_flows.tainted_class_attribute_through_optional_class_issue", + "callable_line": 45, + "code": 5002, + "line": 47, + "start": 8, + "end": 44, + "filename": "class_flows.py", + "message": + "Test flow. Data from [Test] source(s) may reach [Test] sink(s)", + "traces": [ + { + "name": "forward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 47, + "start": 60, + "end": 62 + }, + "leaves": [ { "kind": "Test", "name": "__test_source" } ], + "features": [ { "via": "special_source" } ] + } + ] + }, + { + "name": "backward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 47, + "start": 8, + "end": 44 + }, + "leaves": [ + { + "kind": "Test", + "name": "class_flows.C.__class__.tainted_class_attribute" + } + ] + } + ] + } + ] + } +} +{ + "kind": "issue", + "data": { + "callable": "class_flows.global_class_attribute_issue", + "callable_line": 50, + "code": 5002, + "line": 51, + "start": 4, + "end": 29, + "filename": "class_flows.py", + "message": + "Test flow. Data from [Test] source(s) may reach [Test] sink(s)", + "traces": [ + { + "name": "forward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 51, + "start": 45, + "end": 47 + }, + "leaves": [ { "kind": "Test", "name": "__test_source" } ], + "features": [ { "via": "special_source" } ] + } + ] + }, + { + "name": "backward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 51, + "start": 4, + "end": 29 + }, + "leaves": [ + { + "kind": "Test", + "name": "class_flows.C.__class__.tainted_class_attribute" + } ] } ] @@ -49,10 +154,10 @@ { "kind": "issue", "data": { - "callable": "class_flows.tainted_attribute_for_class", - "callable_line": 23, + "callable": "class_flows.tainted_attribute_flow_issue", + "callable_line": 16, "code": 5002, - "line": 24, + "line": 17, "start": 4, "end": 23, "filename": "class_flows.py", @@ -65,7 +170,7 @@ { "root": { "filename": "class_flows.py", - "line": 24, + "line": 17, "start": 39, "end": 41 }, @@ -80,7 +185,7 @@ { "root": { "filename": "class_flows.py", - "line": 24, + "line": 17, "start": 4, "end": 23 }, @@ -93,3 +198,53 @@ ] } } +{ + "kind": "issue", + "data": { + "callable": "class_flows.tainted_class_attribute_through_class_issue", + "callable_line": 37, + "code": 5002, + "line": 38, + "start": 4, + "end": 40, + "filename": "class_flows.py", + "message": + "Test flow. Data from [Test] source(s) may reach [Test] sink(s)", + "traces": [ + { + "name": "forward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 38, + "start": 56, + "end": 58 + }, + "leaves": [ { "kind": "Test", "name": "__test_source" } ], + "features": [ { "via": "special_source" } ] + } + ] + }, + { + "name": "backward", + "roots": [ + { + "root": { + "filename": "class_flows.py", + "line": 38, + "start": 4, + "end": 40 + }, + "leaves": [ + { + "kind": "Test", + "name": "class_flows.C.__class__.tainted_class_attribute" + } + ] + } + ] + } + ] + } +} diff --git a/taint/test/integration/class_flows.py.pysa b/taint/test/integration/class_flows.py.pysa index 8c1f5546b49..6b8bd04c449 100644 --- a/taint/test/integration/class_flows.py.pysa +++ b/taint/test/integration/class_flows.py.pysa @@ -1,2 +1,3 @@ class_flows.C.tainted_attribute: TaintSink[Test] = ... +class_flows.C.__class__.tainted_class_attribute: TaintSink[Test] = ... def list.append(self, element: TaintInTaintOut[Updates[self]]): ... diff --git a/taint/test/modelTest.ml b/taint/test/modelTest.ml index 608db612c19..13ba5f5d687 100644 --- a/taint/test/modelTest.ml +++ b/taint/test/modelTest.ml @@ -553,6 +553,18 @@ let test_invalid_models context = () +let test_demangle_class_attributes _ = + let assert_demangle ~expected name = + assert_equal expected (Model.demangle_class_attribute name) + in + assert_demangle ~expected:"a.B" "a.B"; + assert_demangle ~expected:"a.B" "a.__class__.B"; + + (* We require `__class__` to directly precede the attribute of the `.`-separated names. *) + assert_demangle ~expected:"a.B.__class__" "a.B.__class__"; + assert_demangle ~expected:"a.__class__.B.C" "a.__class__.B.C" + + let () = "taint_model" >::: [ "attach_features" >:: test_attach_features; @@ -566,5 +578,6 @@ let () = "test_source_breadcrumbs" >:: test_source_breadcrumbs; "test_sink_breadcrumbs" >:: test_sink_breadcrumbs; "test_tito_breadcrumbs" >:: test_tito_breadcrumbs; - "invalid_models" >:: test_invalid_models ] + "invalid_models" >:: test_invalid_models; + "demangle_class_attributes" >:: test_demangle_class_attributes ] |> Test.run diff --git a/test/test.ml b/test/test.ml index 018c3cf393b..6eea36018f5 100644 --- a/test/test.ml +++ b/test/test.ml @@ -505,7 +505,8 @@ let typeshed_stubs ?(include_helper_builtins = true) () = class object(): __doc__: str - def __class__(self, __type: Type[object]) -> None: ... + @property + def __class__(self: _T) -> Type[_T]: ... def __init__(self) -> None: ... def __new__(cls) -> Any: ... def __setattr__(self, name: str, value: Any) -> None: ...