[analyzer][doc] Add user documenation for taint analysis

[gamesh411]

Summary

Checker alpha.security.taint.TaintPropagation now has user documentation for
taint analysis with an example showing external YAML configuration format.
The format of the taint configuration file is now documented under the user
documentation of Clang SA.

[steakhal]

Excellent work. Don't be afraid of the number of nits I dump on you!
Good job.

clang/docs/analyzer/checkers.rst

2338

Same for the rest.

2351

Per https://reviews.llvm.org/D113004#inline-1078695 we should not advocate users use the -Xclang machinery, we should rather refer to it by other tools such as scan-build. However, we haven't reached a consensus about this decision yet.
Consider moving some parts of this doc to the proposed Configuration documentation file - housing the more user-facing analyzer options.

2356

Please note that this configuration file is not stable (yet).
We might change it in a non-backward compatible way without any notice in the upcoming releases.

clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

19

41–51

Since there the SrcArgs is empty, fread will work as an unconditional propagator - aka. a taint source.
It's probably better to phrase it something like that.

By doing so you could signify that in the next example you have a non-empty SrcArgs list, thus the propagation is conditional.

I think it would be better to follow this pattern in the corresponding section later as well.

49

76

The literature probably refers to cleansing functions as sanitizers. Wikipedia

89

Previously it was referred to by the key word. Chose one for consistency.

89

I would rather use the operation or function call instead of event everywhere.

91

92

This sentence definitely needs some polishing.

1. It should emphasize that a source is an unconditional propagation - at least in the current design.
2. The user might want to enlist or mark a specific function as taint source but definitely not identify them.

94

102

109

114

In the following section, you have an empty line after this marker line. Consolidate these.

117

You sometimes use parameter and in other cases argument seemingly interchangeably. You should consider consolidating these.

133

It's not exactly for this patch, but we should investigate If we could infer this index from the declaration of the function.

[gamesh411]

clang/docs/analyzer/checkers.rst

2351

Removed the command-line invocation part, and just left a mention to the configuration option.

clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

133

Good idea, this seems like the way forward, I agree.

[gamesh411]

Fix the review comments of @steakhal

[whisperity]

clang/docs/analyzer/checkers.rst

2341–2342

What does it mean that these are "default propagations"? That taint passes through calls to them trivially?

2345

execvp and execvP? What is the capital P for? I can't find this overload in the POSIX docs.

2353

Perhaps we should link to the in-house YAML doc (http://llvm.org/docs/YamlIO.html#introduction-to-yaml) first, and have the user move to other sources from there?

clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

5

The Clang Static [...]?

5

uses, or can use?

6

Clang -> Clang SA / CSA?

6

So the checker has the defaults all the time, or only by enabling the alias can I get the defaults?

7

Ditto about YAML.

13

I think this wanted to be an anchor? Also see how the syntax highlighting broke in the next section.

23

AFAIK, RST anchors are global identifiers for the project, so maybe we should take care in adding "clang-sa" or some other sort of namespaceing...

(Although this might depend on how exactly the docs are built!)

30

Filters are not mentioned earlier.

31–36

If none of the comments themselves are just commented out ("optional") keys in the YAML, it would be better readable with a bit of formatting.

41

Why is this comment at indent 0?

51

What are these magic numbers and what is their meaning?

125–126

Ensure that the numeric literal shows up as "code" with double backtick.

[gamesh411]

tidy up based on comments from whispy

clang/docs/analyzer/checkers.rst

2341–2342

Added just 2 sentences about what default means.
Please see them a bit further up in the new version of the patch.

2345

I have found a refence to this function inside TargetLibraryInfoTest.cpp.

926   │   case LibFunc_execv:
927   │   case LibFunc_execvp:
928   │     return (NumParams == 2 && FTy.getParamType(0)->isPointerTy() &&
929   │             FTy.getParamType(1)->isPointerTy() &&
930   │             FTy.getReturnType()->isIntegerTy(32));
931   │   case LibFunc_execvP:
932   │   case LibFunc_execvpe:
933   │   case LibFunc_execve:
934   │     return (NumParams == 3 && FTy.getParamType(0)->isPointerTy() &&
935   │             FTy.getParamType(1)->isPointerTy() &&
936   │             FTy.getParamType(2)->isPointerTy() &&
937   │             FTy.getReturnType()->isIntegerTy(32));

It seems like an OSX-specific spelling and seems like it has the semantics of execve.
During the implementation of this patch, I just collected the references to functions inside GenericTaintChecker.cpp.

clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

5

For brevity and clarity, I will leave it at uses, it is just a generic statement about the availability of taint analysis as a method, and I think it would not improve the understanding to change it to can use. IMHO it is implicit, and can use would make people wonder as to what does Clang need to use it etc.

6

Good point, added a line to clarify this.

23

added clangsa- prefix

30

Added a mention with references in the Overview section.

31–36

Reformatted the examples section according to your suggestions.

41

It is because Propagations YAML-keys are used to define both sources and propagations. So to comment the "source-section" 0-indent is used, and to comment on individual entries within, 2-indent comments are used.

I have consolidated the indents and introduced vertical whitespace instead of multiple indentation levels.

51

Added an extra comment on top of the example YAML file.

[gamesh411]

fix indentation warning
make inline code formatting look better

[gamesh411]

update with arc diff $(git merge-base HEAD upstream) --update D113251
in order to satisfy workflow pre-merge checks

clang/docs/analyzer/checkers.rst

int test(void) {
    int x;
    return x; // warn: undefined or garbage returned
  }
 
 .. _alpha-security-taint-TaintPropagation:
 
 alpha.security.taint.TaintPropagation (C, C++)
 """"""""""""""""""""""""""""""""""""""""""""""
-Generate taint information used by other checkers.
-A data is tainted when it comes from an unreliable source.
+Taint analysis identifies untrusted sources of information (taint sources), rules as to how the untrusted data flows along the execution path (propagation rules), and points of execution where the use of tainted data is risky (taints sinks).
+The most notable examples of taint sources are:
+  - network originating data
+  - environment variables
+  - database originating data
+
+``GenericTaintChecker`` is the main implementation checker for this rule, and it generates taint information used by other checkers.
 
 .. code-block:: c
 
  void test() {
    char x = getchar(); // 'x' marked as tainted
    system(&x); // warn: untrusted data is passed to a system call
  }
 

 
  void test() {
    size_t ts;
    scanf("%zd", &ts); // 'ts' marked as tainted
    int *p = (int *)malloc(ts * sizeof(int));
      // warn: untrusted data as buffer size
  }
 
+Default sources defined by `GenericTaintChecker`:
+``fdopen``, ``fopen``, ``freopen``, ``getch``, ``getchar``, ``getchar_unlocked``, ``gets``, ``scanf``, ``socket``, ``wgetch``
+
+Default propagations defined by `GenericTaintChecker`:
+``atoi``, ``atol``, ``atoll``, ``fgetc``, ``fgetln``, ``fgets``, ``fscanf``, ``sscanf``, ``getc``, ``getc_unlocked``, ``getdelim``, ``getline``, ``getw``, ``pread``, ``read``, ``strchr``, ``strrchr``, ``tolower``, ``toupper``
+
+Default sinks defined in `GenericTaintChecker`:
+``printf``, ``setproctitle``, ``system``, ``popen``, ``execl``, ``execle``, ``execlp``, ``execv``, ``execvp``, ``execvP``, ``execve``, ``dlopen``, ``memcpy``, ``memmove``, ``strncpy``, ``strndup``, ``malloc``, ``calloc``, ``alloca``, ``memccpy``, ``realloc``, ``bcopy``
+
+The user can configure taint sources, sinks, and propagation rules by providing a configuration file via checker option ``alpha.security.taint.TaintPropagation:Config``.
+
+.. code-block:: sh
+
+ clang --analyze ... -Xclang -analyzer-config -Xclang alpha.security.taint.TaintPropagation:Config=taint_config.yaml
+
+External taint configuration is in `YAML <https://yaml.org/>`_ format. The taint-related options defined in the config file extend but do not override the built-in sources, rules, sinks.
+
+
+For a more detailed description of configuration options, please see the :doc:`user-docs/TaintAnalysisConfiguration`. For an example see :ref:`taint-configuration-example`.
+
 alpha.unix
 ^^^^^^^^^^^
 
 .. _alpha-unix-BlockInCriticalSection:
 
 alpha.unix.BlockInCriticalSection (C)
 """""""""""""""""""""""""""""""""""""
 Check for calls to blocking functions inside a critical section.

clang/docs/analyzer/user-docs.rst

 User Docs
 =========
 
 Contents:
 
 .. toctree::
    :maxdepth: 2
 
    user-docs/CrossTranslationUnit
+   user-docs/TaintAnalysisConfiguration

clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

+============================
+Taint Analysis Configuration
+============================
+
+Clang Static Analyzer uses taint analysis to detect security-related issues in code.
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which the user can access via the :ref:`alpha-security-taint-TaintPropagation` checker alias and this checker has a default taint-related configuration.
+The checker also provides a configuration interface for extending the default settings by providing a configuration file in `YAML <https://yaml.org/>`_ format.
+This documentation describes the syntax of the configuration file and gives the informal semantics of the configuration options.
+
+.. contents::
+   :local:
+
+.. _taint-configuration-overview
+
+Overview
+________
+
+Taint analysis works by checking for the occurrence of special events during the symbolic execution of the program.
+Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates in a taint source, touches a taint sink, and propagates through the program paths via propagation rules.
+A source, sink, or an event that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by :ref:`alpha-security-taint-TaintPropagation`.
+The checker's documentation also specifies how to provide a custom taint configuration with command-line options.
+
+.. _taint-configuration-example:
+
+Example configuration file
+__________________________
+
+.. code-block:: yaml
+
+  Filters:
+    # signature:
+    # void cleanse_first_arg(int* arg)
+    #
+    # example:
+    # int x; // x is tainted
+    # cleanse_first_arg(&x); // x is not tainted anymore
+    - Name: cleanse_first_arg
+      Args: [0]
+
+  Propagations:
+  # sources:
+    # signature:
+    # size_t fread(void *ptr, size_t size, size_t nmemb, FILE * stream)
+    #
+    # example:
+    # FILE* f = fopen("file.txt");
+    # char buf[1024];
+    # size_t read = fread(buf, sizeof(buf[0]), sizeof(buf)/sizeof(buf[0]), f);
+    # // read and buf is tainted
+    - Name: fread
+      DstArgs: [0, -1]
+
+  # propagations:
+    # signature:
+    # char *dirname(char *path)
+    #
+    # example:
+    # char* path = read_path();
+    # char* dir = dirname(path);
+    # // dir is tainted if path was tainted
+    - Name: dirname
+      SrcArgs: [0]
+      DstArgs: [-1]
+
+  Sinks:
+    # siganture:
+    # int system(const char* command)
+    #
+    # example:
+    # const char* command = read_command();
+    # system(command); // emit diagnostic if command is tainted
+    - Name: system
+      Args: [0]
+
+In the example file above, the entries under the `Propagation` key implement the conceptual sources and propagations, and sinks have their dedicated `Sinks` key.
+The user can define program points where the tainted values should be cleansed by listing entries under the `Filters` key.
+Filters model the sanitization of values done by the programmer, and providing these is key to avoiding false-positive findings.
+
+Configuration file syntax and semantics
+_______________________________________
+
+The configuration file should have valid `YAML <https://yaml.org/>`_ syntax.
+
+The configuration file can have the following top-level keys:
+ - Filters
+ - Propagations
+ - Sinks
+
+Under the `Filters` entry, the user can specify a list of events that remove taint (see :ref:`taint-filter-details` for details).
+
+Under the `Propagations` entry, the user can specify a list of events that generate and propagate taint (see :ref:`taint-propagation-details` for details).
+The user can identify taint sources with a `SrcArgs` key in the `Propagation` entry, while propagations have none.
+
+Under the `Sinks` entry, the user can specify a list of events where the checker should emit a bug report if taint reaches there (see :ref:`taint-sink-details` for details).
+
+.. _taint-filter-details:
+
+Filter syntax and semantics
+###########################
+An entry under `Filters` is a `YAML <https://yaml.org/>`_ object with the following mandatory keys:
+ - `Name` is a string that specifies the name of a function.
+   Encountering this function during symbolic execution will clean taint on some parameters or the return value.
+ - `Args` is a list of numbers in the range of [-1..int_max].
+   It indicates the indexes of arguments in the function call event.
+   The number -1 signifies the return value; other numbers identify call arguments.
+   The values of these arguments are considered clean after the function call.
+
+The following keys are optional:
+ - `Scope` is a string that specifies the prefix of the function's name in its fully qualified name. This option restricts the set of matching function calls.
+
+ .. _taint-propagation-details:
+
+Propagation syntax and semantics
+################################
+An entry under `Propagation` is a `YAML <https://yaml.org/>`_ object with the following mandatory keys:
+ - `Name` is a string that specifies the name of a function.
+   Encountering this function during symbolic execution propagate taint from one or more parameters to other parameters and possibly the return value.
+   It helps model the taint-related behavior of functions that are not analyzable otherwise.
+
+The following keys are optional:
+ - `Scope` is a string that specifies the prefix of the function's name in its fully qualified name. This option restricts the set of matching function calls.
+ - `SrcArgs` is a list of numbers in the range of [0..int_max] that indicates the indexes of arguments in the function call event.
+   Taint-propagation considers the values of these arguments during the evaluation of the function call.
+   If any `SrcArgs` arguments are tainted, the checker will consider all `DstArgs` arguments tainted after the call.
+ - `DstArgs` is a list of numbers in the range of [-1..int_max] that indicates the indexes of arguments in the function call event.
+   The number -1 specifies the return value of the function.
+   If any `SrcArgs` arguments are tainted, the checker will consider all `DstArgs` arguments tainted after the call.
+ - `VariadicType` is a string that can be one of ``None``, ``Dst``, ``Src``.
+   It is used in conjunction with `VariadicIndex` to specify arguments inside a variadic argument.
+   The value of ``Src`` will treat every call site argument that is part of a variadic argument list as a source concerning propagation rules (as if specified by `SrcArg`).
+   The value of ``Dst`` will treat every call site argument that is part of a variadic argument list a destination concerning propagation rules.
+   The value of ``None`` will not consider the arguments that are part of a variadic argument list (this option is redundant but can be used to temporarily switch off handling of a particular variadic argument option without removing the entire variadic entry).
+ - `VariadicIndex` is a number in the range of [0..int_max]. It indicates the starting index of the variadic argument in the signature of the function.
+
+
+.. _taint-sink-details:
+
+Sink syntax and semantics
+#########################
+
+An entry under `Sinks` is a `YAML <https://yaml.org/>`_ object with the following mandatory keys:
+ - `Name` is a string that specifies the name of a function.
+   Encountering this function during symbolic execution will emit a taint-related diagnostic if any of the arguments specified with `Args` are tainted at the call site.
+ - `Args` is a list of numbers in the range of [0..int_max] that indicates the indexes of arguments in the function call event.
+   The checker reports an error if any of the specified arguments are tainted.
+
+The following keys are optional:
+ - `Scope` is a string that specifies the prefix of the function's name in its fully qualified name. This option restricts the set of matching function calls.