document Quandary issue types

Summary: Most of them had some form of documentation in source comments. Reviewed By: ngorogiannis Differential Revision: D22020016 fbshipit-source-id: 468f86658
5 years ago · b90f0a0182
parent b6c8e52e8c
commit b90f0a0182
3 changed files with 74 additions and 39 deletions
--- a/infer/src/base/IssueType.ml
+++ b/infer/src/base/IssueType.ml
@ -414,9 +414,17 @@ let constant_address_dereference =
    ~user_documentation:[%blob "../../documentation/issues/CONSTANT_ADDRESS_DEREFERENCE.md"]


-let create_intent_from_uri = register_from_string ~id:"CREATE_INTENT_FROM_URI" Error Quandary
+let create_intent_from_uri =
+  register_from_string ~id:"CREATE_INTENT_FROM_URI" Error Quandary
+    ~user_documentation:
+      "Create an intent/start a component using a (possibly user-controlled) URI. may or may not \
+       be an issue depending on where the URI comes from."
+
+
+let cross_site_scripting =
+  register_from_string ~id:"CROSS_SITE_SCRIPTING" Error Quandary
+    ~user_documentation:"Untrusted data flows into HTML; XSS risk."

-let cross_site_scripting = register_from_string ~id:"CROSS_SITE_SCRIPTING" Error Quandary

 let _cxx_reference_captured_in_objc_block =
  register_from_string ~id:"CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK" Warning Linters
@ -591,6 +599,7 @@ let eradicate_meta_class_can_be_nullsafe =

 let exposed_insecure_intent_handling =
  register_from_string ~id:"EXPOSED_INSECURE_INTENT_HANDLING" Error Quandary
+    ~user_documentation:"Undocumented."


 let failure_exe = register_from_string ~visibility:Silent ~id:"Failure_exe" Info Biabduction
@ -658,7 +667,10 @@ let inherently_dangerous_function =
  register_from_string ~visibility:Developer ~id:"INHERENTLY_DANGEROUS_FUNCTION" Warning Biabduction


-let insecure_intent_handling = register_from_string ~id:"INSECURE_INTENT_HANDLING" Error Quandary
+let insecure_intent_handling =
+  register_from_string ~id:"INSECURE_INTENT_HANDLING" Error Quandary
+    ~user_documentation:"Undocumented."
+

 let integer_overflow_l1 =
  register_from_string ~id:"INTEGER_OVERFLOW_L1" Error BufferOverrunChecker
@ -696,7 +708,10 @@ let internal_error =

 let invariant_call = register_from_string ~enabled:false ~id:"INVARIANT_CALL" Error LoopHoisting

-let javascript_injection = register_from_string ~id:"JAVASCRIPT_INJECTION" Error Quandary
+let javascript_injection =
+  register_from_string ~id:"JAVASCRIPT_INJECTION" Error Quandary
+    ~user_documentation:"Untrusted data flows into JavaScript."
+

 let lab_resource_leak = register_from_string ~id:"LAB_RESOURCE_LEAK" Error ResourceLeakLabExercise

@ -723,7 +738,9 @@ let lockless_violation =
    ~user_documentation:[%blob "../../documentation/issues/LOCKLESS_VIOLATION.md"]


-let logging_private_data = register_from_string ~id:"LOGGING_PRIVATE_DATA" Error Quandary
+let logging_private_data =
+  register_from_string ~id:"LOGGING_PRIVATE_DATA" Error Quandary ~user_documentation:"Undocumented."
+

 let expensive_loop_invariant_call =
  register_from_string ~id:"EXPENSIVE_LOOP_INVARIANT_CALL" Error LoopHoisting
@ -807,6 +824,7 @@ let pure_function = register_from_string ~id:"PURE_FUNCTION" Error Purity

 let quandary_taint_error =
  register_from_string ~hum:"Taint Error" ~id:"QUANDARY_TAINT_ERROR" Error Quandary
+    ~user_documentation:"Generic taint error when nothing else fits."


 let _registered_observer_being_deallocated =
@ -833,13 +851,25 @@ let skip_pointer_dereference =
  register_from_string ~enabled:false ~id:"SKIP_POINTER_DEREFERENCE" Info Biabduction


-let shell_injection = register_from_string ~id:"SHELL_INJECTION" Error Quandary
+let shell_injection =
+  register_from_string ~id:"SHELL_INJECTION" Error Quandary
+    ~user_documentation:"Environment variable or file data flowing to shell."
+
+
+let shell_injection_risk =
+  register_from_string ~id:"SHELL_INJECTION_RISK" Error Quandary
+    ~user_documentation:"Code injection if the caller of the endpoint doesn't sanitize on its end."
+
+
+let sql_injection =
+  register_from_string ~id:"SQL_INJECTION" Error Quandary
+    ~user_documentation:"Untrusted and unescaped data flows to SQL."

-let shell_injection_risk = register_from_string ~id:"SHELL_INJECTION_RISK" Error Quandary

-let sql_injection = register_from_string ~id:"SQL_INJECTION" Error Quandary
+let sql_injection_risk =
+  register_from_string ~id:"SQL_INJECTION_RISK" Error Quandary
+    ~user_documentation:"Untrusted and unescaped data flows to SQL."

-let sql_injection_risk = register_from_string ~id:"SQL_INJECTION_RISK" Error Quandary

 let stack_variable_address_escape =
  register_from_string ~id:"STACK_VARIABLE_ADDRESS_ESCAPE" Error Pulse
@ -919,36 +949,64 @@ let use_after_lifetime =
    ~user_documentation:[%blob "../../documentation/issues/USE_AFTER_LIFETIME.md"]


-let user_controlled_sql_risk = register_from_string ~id:"USER_CONTROLLED_SQL_RISK" Error Quandary
+let user_controlled_sql_risk =
+  register_from_string ~id:"USER_CONTROLLED_SQL_RISK" Error Quandary
+    ~user_documentation:"Untrusted data flows to SQL (no injection risk)."
+

 let untrusted_buffer_access =
  register_from_string ~enabled:false ~id:"UNTRUSTED_BUFFER_ACCESS" Error Quandary
+    ~user_documentation:"Untrusted data of any kind flowing to buffer."
+

+let untrusted_deserialization =
+  register_from_string ~id:"UNTRUSTED_DESERIALIZATION" Error Quandary
+    ~user_documentation:"User-controlled deserialization."

-let untrusted_deserialization = register_from_string ~id:"UNTRUSTED_DESERIALIZATION" Error Quandary

 let untrusted_deserialization_risk =
  register_from_string ~id:"UNTRUSTED_DESERIALIZATION_RISK" Error Quandary
+    ~user_documentation:"User-controlled deserialization"


 let untrusted_environment_change_risk =
  register_from_string ~id:"UNTRUSTED_ENVIRONMENT_CHANGE_RISK" Error Quandary
+    ~user_documentation:"User-controlled environment mutation."
+
+
+let untrusted_file =
+  register_from_string ~id:"UNTRUSTED_FILE" Error Quandary
+    ~user_documentation:
+      "User-controlled file creation; may be vulnerable to path traversal and more."


-let untrusted_file = register_from_string ~id:"UNTRUSTED_FILE" Error Quandary
+let untrusted_file_risk =
+  register_from_string ~id:"UNTRUSTED_FILE_RISK" Error Quandary
+    ~user_documentation:
+      "User-controlled file creation; may be vulnerable to path traversal and more."

-let untrusted_file_risk = register_from_string ~id:"UNTRUSTED_FILE_RISK" Error Quandary

 let untrusted_heap_allocation =
  register_from_string ~enabled:false ~id:"UNTRUSTED_HEAP_ALLOCATION" Error Quandary
+    ~user_documentation:
+      "Untrusted data of any kind flowing to heap allocation. this can cause crashes or DOS."
+

+let untrusted_intent_creation =
+  register_from_string ~id:"UNTRUSTED_INTENT_CREATION" Error Quandary
+    ~user_documentation:"Creating an Intent from user-controlled data."

-let untrusted_intent_creation = register_from_string ~id:"UNTRUSTED_INTENT_CREATION" Error Quandary

-let untrusted_url_risk = register_from_string ~id:"UNTRUSTED_URL_RISK" Error Quandary
+let untrusted_url_risk =
+  register_from_string ~id:"UNTRUSTED_URL_RISK" Error Quandary
+    ~user_documentation:"Untrusted flag, environment variable, or file data flowing to URL."
+

 let untrusted_variable_length_array =
  register_from_string ~id:"UNTRUSTED_VARIABLE_LENGTH_ARRAY" Error Quandary
+    ~user_documentation:
+      "Untrusted data of any kind flowing to stack buffer allocation. Trying to allocate a stack \
+       buffer that's too large will cause a stack overflow."


 let vector_invalidation = register_from_string ~id:"VECTOR_INVALIDATION" Error Pulse
--- a/infer/src/quandary/ClangTrace.ml
+++ b/infer/src/quandary/ClangTrace.ml
@ -482,19 +482,13 @@ include TaintTrace.Make (struct
    | (CommandLineFlag (_, typ) | Endpoint (_, typ) | UserControlledEndpoint (_, typ)), SQLInjection
      ->
        if is_injection_possible ~typ Sanitizer.EscapeSQL sanitizers then
-          (* SQL injection if the caller of the endpoint doesn't sanitize on its end *)
          Some IssueType.sql_injection_risk
-        else
-          (* no injection risk, but still user-controlled *)
-          Some IssueType.user_controlled_sql_risk
+        else Some IssueType.user_controlled_sql_risk
    | (Endpoint _ | UserControlledEndpoint _), (SQLRead | SQLWrite) ->
-        (* no injection risk, but still user-controlled *)
        Some IssueType.user_controlled_sql_risk
    | (Endpoint _ | UserControlledEndpoint _), EnvironmentChange ->
-        (* user-controlled environment mutation *)
        Some IssueType.untrusted_environment_change_risk
    | (CommandLineFlag (_, typ) | Endpoint (_, typ) | UserControlledEndpoint (_, typ)), ShellExec ->
-        (* code injection if the caller of the endpoint doesn't sanitize on its end *)
        Option.some_if
          (is_injection_possible ~typ Sanitizer.EscapeShell sanitizers)
          IssueType.shell_injection_risk
@ -505,10 +499,8 @@ include TaintTrace.Make (struct
        | ReadFile
        | Other )
      , BufferAccess ) ->
-        (* untrusted data of any kind flowing to buffer *)
        Some IssueType.untrusted_buffer_access
    | (EnvironmentVariable | ReadFile | Other), ShellExec ->
-        (* environment var, or file data flowing to shell *)
        Option.some_if
          (is_injection_possible Sanitizer.EscapeShell sanitizers)
          IssueType.shell_injection
@ -518,7 +510,6 @@ include TaintTrace.Make (struct
          (is_injection_possible Sanitizer.EscapeSQL sanitizers)
          IssueType.sql_injection
    | Other, URL ->
-        (* untrusted flag, environment var, or file data flowing to URL *)
        Option.some_if
          (is_injection_possible Sanitizer.EscapeURL sanitizers)
          IssueType.untrusted_url_risk
@ -529,7 +520,6 @@ include TaintTrace.Make (struct
        | ReadFile
        | Other )
      , HeapAllocation ) ->
-        (* untrusted data of any kind flowing to heap allocation. this can cause crashes or DOS. *)
        Some IssueType.untrusted_heap_allocation
    | ( ( CommandLineFlag _
        | Endpoint _
@ -538,8 +528,6 @@ include TaintTrace.Make (struct
        | ReadFile
        | Other )
      , StackAllocation ) ->
-        (* untrusted data of any kind flowing to stack buffer allocation. trying to allocate a stack
-           buffer that's too large will cause a stack overflow. *)
        Some IssueType.untrusted_variable_length_array
    | ( (CommandLineFlag _ | EnvironmentVariable | ReadFile)
      , (CreateFile | EnvironmentChange | SQLRead | SQLWrite | URL) ) ->
--- a/infer/src/quandary/JavaTrace.ml
+++ b/infer/src/quandary/JavaTrace.ml
@ -581,34 +581,25 @@ include TaintTrace.Make (struct
      ->
        None
    | (Endpoint _ | Intent | UserControlledString | UserControlledURI), CreateIntent ->
-        (* creating Intent from user-congrolled data *)
        Some IssueType.untrusted_intent_creation
    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), CreateFile ->
-        (* user-controlled file creation; may be vulnerable to path traversal + more *)
        Some IssueType.untrusted_file
    | Endpoint _, CreateFile ->
-        (* user-controlled file creation; may be vulnerable to path traversal + more *)
        Some IssueType.untrusted_file_risk
    | (Intent | IntentFromURI | UserControlledString | UserControlledURI), Deserialization ->
-        (* shouldn't let anyone external control what we deserialize *)
        Some IssueType.untrusted_deserialization
    | Endpoint _, Deserialization ->
-        (* shouldn't let anyone external control what we deserialize *)
        Some IssueType.untrusted_deserialization_risk
    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), HTML ->
-        (* untrusted data flows into HTML; XSS risk *)
        Some IssueType.cross_site_scripting
    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), JavaScript
      ->
-        (* untrusted data flows into JS *)
        Some IssueType.javascript_injection
    | (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI), SQLInjection
      ->
-        (* untrusted and unescaped data flows to SQL *)
        Some IssueType.sql_injection_risk
    | ( (Endpoint _ | Intent | IntentFromURI | UserControlledString | UserControlledURI)
      , (SQLRead | SQLWrite) ) ->
-        (* untrusted data flows to SQL *)
        Some IssueType.user_controlled_sql_risk
    | DrawableResource _, OpenDrawableResource ->
        (* not a security issue, but useful for debugging flows from resource IDs to inflation *)
@ -618,8 +609,6 @@ include TaintTrace.Make (struct
    | IntentForInsecureIntentHandling {exposed= false}, StartComponentForInsecureIntentHandling ->
        Some IssueType.insecure_intent_handling
    | IntentFromURI, StartComponent ->
-        (* create an intent/start a component using a (possibly user-controlled) URI. may or may not
-           be an issue; depends on where the URI comes from *)
        Some IssueType.create_intent_from_uri
    | PrivateData, Logging ->
        Some IssueType.logging_private_data