[analyzer] Taint is not being applied to classes, only class members

[tomrittervg]

This might be working as intended, but just to be sure.

C++ file

class PortClass 
{
public:
    int foo;
};

void clang_analyzer_isTainted(int);
void clang_analyzer_isTainted(PortClass);

int ThisFunctionReturnsSomethingTainted1();
PortClass ThisFunctionReturnsSomethingTainted2();

template<typename T>
T ReadPrivilegedParam();


void foo()
{
    int port1 = ThisFunctionReturnsSomethingTainted1();
    clang_analyzer_isTainted(port1); // Tainted, as expected

    int port2 = ReadPrivilegedParam<int>();
    clang_analyzer_isTainted(port2); // Tainted, as expected

    PortClass port3 = ThisFunctionReturnsSomethingTainted2();
    clang_analyzer_isTainted(port3); // Not tainted ???
    clang_analyzer_isTainted(port3.foo); // Tainted...

    PortClass port4 = ReadPrivilegedParam<PortClass>();
    clang_analyzer_isTainted(port4); // Not tainted ???
    clang_analyzer_isTainted(port4.foo); // Tainted...
}

taint config file:

Propagations:
  - Name: ReadPrivilegedParam
    DstArgs: [-1]

  - Name: privilegedextract
    DstArgs: [-1]

  - Name: ThisFunctionReturnsSomethingTainted1
    DstArgs: [-1]  

  - Name: ThisFunctionReturnsSomethingTainted2
    DstArgs: [-1]  

Commands:

#!/bin/bash

echo "Generating AST"
clang-20 \
-c \
-x c++ \
-emit-ast \
-D__clang_analyzer__ \
-w \
-o repro.cpp.ast \
repro.cpp

touch externalDefMap.txt

echo "extdef mapping"
clang-extdef-mapping \
repro.cpp \
-- \
-c \
-x c++ \
>> externalDefMap.txt 

echo "Analyzing"
clang-20 \
--analyze \
-Qunused-arguments \
-Xclang -analyzer-opt-analyze-headers \
-Xclang -analyzer-config \
-Xclang expand-macros=true \
-Xclang -analyzer-config \
-Xclang optin.taint.TaintPropagation:Config=myconfig.yaml \
-Xclang -analyzer-checker=debug.TaintTest,debug.ExprInspection,optin.taint.TaintedAlloc,optin.taint.TaintedDiv,optin.taint.GenericTaint \
-Xclang -analyzer-config \
-Xclang ctu-dir=. \
-Xclang -analyzer-config \
-Xclang display-ctu-progress=true \
-x c++ \
repro.cpp 

This is run from a version of clang built from git on approximately October 9th

Output:

repro.cpp:19:17: warning: tainted [debug.TaintTest]
   19 |     int port1 = ThisFunctionReturnsSomethingTainted1();
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:20:5: warning: YES [debug.ExprInspection]
   20 |     clang_analyzer_isTainted(port1); // Tainted, as expected
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:20:30: warning: tainted [debug.TaintTest]
   20 |     clang_analyzer_isTainted(port1); // Tainted, as expected
      |                              ^~~~~
repro.cpp:22:17: warning: tainted [debug.TaintTest]
   22 |     int port2 = ReadPrivilegedParam<int>();
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:23:5: warning: YES [debug.ExprInspection]
   23 |     clang_analyzer_isTainted(port2); // Tainted, as expected
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:23:30: warning: tainted [debug.TaintTest]
   23 |     clang_analyzer_isTainted(port2); // Tainted, as expected
      |                              ^~~~~
repro.cpp:26:5: warning: NO [debug.ExprInspection] <---------------------------------------------
   26 |     clang_analyzer_isTainted(port3); // Not tainted ???
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:27:5: warning: YES [debug.ExprInspection]
   27 |     clang_analyzer_isTainted(port3.foo); // Tainted...
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:27:30: warning: tainted [debug.TaintTest]
   27 |     clang_analyzer_isTainted(port3.foo); // Tainted...
      |                              ^~~~~~~~~
repro.cpp:30:5: warning: NO [debug.ExprInspection] <---------------------------------------------
   30 |     clang_analyzer_isTainted(port4); // Not tainted ???
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:31:5: warning: YES [debug.ExprInspection]
   31 |     clang_analyzer_isTainted(port4.foo); // Tainted...
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:31:30: warning: tainted [debug.TaintTest]
   31 |     clang_analyzer_isTainted(port4.foo); // Tainted...
      |                              ^~~~~~~~~
12 warnings generated.

cc @llvm/issue-subscribers-clang-static-analyzer

[llvmbot]

@llvm/issue-subscribers-clang-static-analyzer

Author: Tom Ritter (tomrittervg)

This might be working as intended, but just to be sure.

C++ file

class PortClass 
{
public:
    int foo;
};

void clang_analyzer_isTainted(int);
void clang_analyzer_isTainted(PortClass);

int ThisFunctionReturnsSomethingTainted1();
PortClass ThisFunctionReturnsSomethingTainted2();

template<typename T>
T ReadPrivilegedParam();


void foo()
{
    int port1 = ThisFunctionReturnsSomethingTainted1();
    clang_analyzer_isTainted(port1); // Tainted, as expected

    int port2 = ReadPrivilegedParam<int>();
    clang_analyzer_isTainted(port2); // Tainted, as expected

    PortClass port3 = ThisFunctionReturnsSomethingTainted2();
    clang_analyzer_isTainted(port3); // Not tainted ???
    clang_analyzer_isTainted(port3.foo); // Tainted...

    PortClass port4 = ReadPrivilegedParam<PortClass>();
    clang_analyzer_isTainted(port4); // Not tainted ???
    clang_analyzer_isTainted(port4.foo); // Tainted...
}

taint config file:

Propagations:
  - Name: ReadPrivilegedParam
    DstArgs: [-1]

  - Name: privilegedextract
    DstArgs: [-1]

  - Name: ThisFunctionReturnsSomethingTainted1
    DstArgs: [-1]  

  - Name: ThisFunctionReturnsSomethingTainted2
    DstArgs: [-1]  

Commands:

#!/bin/bash

echo "Generating AST"
clang-20 \
-c \
-x c++ \
-emit-ast \
-D__clang_analyzer__ \
-w \
-o repro.cpp.ast \
repro.cpp

touch externalDefMap.txt

echo "extdef mapping"
clang-extdef-mapping \
repro.cpp \
-- \
-c \
-x c++ \
>> externalDefMap.txt 

echo "Analyzing"
clang-20 \
--analyze \
-Qunused-arguments \
-Xclang -analyzer-opt-analyze-headers \
-Xclang -analyzer-config \
-Xclang expand-macros=true \
-Xclang -analyzer-config \
-Xclang optin.taint.TaintPropagation:Config=myconfig.yaml \
-Xclang -analyzer-checker=debug.TaintTest,debug.ExprInspection,optin.taint.TaintedAlloc,optin.taint.TaintedDiv,optin.taint.GenericTaint \
-Xclang -analyzer-config \
-Xclang ctu-dir=. \
-Xclang -analyzer-config \
-Xclang display-ctu-progress=true \
-x c++ \
repro.cpp 

This is run from a version of clang built from git on approximately October 9th

Output:

repro.cpp:19:17: warning: tainted [debug.TaintTest]
   19 |     int port1 = ThisFunctionReturnsSomethingTainted1();
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:20:5: warning: YES [debug.ExprInspection]
   20 |     clang_analyzer_isTainted(port1); // Tainted, as expected
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:20:30: warning: tainted [debug.TaintTest]
   20 |     clang_analyzer_isTainted(port1); // Tainted, as expected
      |                              ^~~~~
repro.cpp:22:17: warning: tainted [debug.TaintTest]
   22 |     int port2 = ReadPrivilegedParam<int>();
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:23:5: warning: YES [debug.ExprInspection]
   23 |     clang_analyzer_isTainted(port2); // Tainted, as expected
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:23:30: warning: tainted [debug.TaintTest]
   23 |     clang_analyzer_isTainted(port2); // Tainted, as expected
      |                              ^~~~~
repro.cpp:26:5: warning: NO [debug.ExprInspection] <---------------------------------------------
   26 |     clang_analyzer_isTainted(port3); // Not tainted ???
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:27:5: warning: YES [debug.ExprInspection]
   27 |     clang_analyzer_isTainted(port3.foo); // Tainted...
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:27:30: warning: tainted [debug.TaintTest]
   27 |     clang_analyzer_isTainted(port3.foo); // Tainted...
      |                              ^~~~~~~~~
repro.cpp:30:5: warning: NO [debug.ExprInspection] <---------------------------------------------
   30 |     clang_analyzer_isTainted(port4); // Not tainted ???
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:31:5: warning: YES [debug.ExprInspection]
   31 |     clang_analyzer_isTainted(port4.foo); // Tainted...
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
repro.cpp:31:30: warning: tainted [debug.TaintTest]
   31 |     clang_analyzer_isTainted(port4.foo); // Tainted...
      |                              ^~~~~~~~~
12 warnings generated.

cc @llvm/issue-subscribers-clang-static-analyzer

[NagyDonat]

Unfortunately the data model of the static analyzer is a bit low-level, and support for objects/instances tend to be limited to representing class-typed lvalues as memory regions which contains subregions corresponding to the data members. In theory the types CompoudVal and LazyCompoundVal provide some support for instance rvalues, but those are underutilized and difficult to understand. The engine usually ensures that constructors are called and e.g. a defaulted copy constructor properly initializes the data members, but it doesn't record a special relationship between the original object (as a single unit) and its copy.

As a part of this trend, the taint propagation checker also focuses on the presence of taint within concrete low-level regions (e.g. foo in your example). AFAIK it has a feature which models that the subregion of a tainted region is also tainted, but presumably it doesn't have analogous logic in the opposite direction (I didn't check this, but your bug report strongly implies it).

I think it would be nice to improve the modeling of instances in the taint handling logic (and elsewhere) and I don't think that there are any fundamental obstacles -- just that this is a complicated area (especially with some C++ features) and we simply didn't get to working on it.

I'm dropping this issue into the backlog of our team. If nobody else solves it, perhaps we can improve this area when we'll have more time (perhaps around the end of the year?).

[steakhal]

I'm surprised it doesn't work out of the box. The fact that the sub-region foo is recognized tainted underpins that we correctly registered the whole object being tainted. I think, simply the query function is bugged, and fails to realize this.

Yes, LazyCompoundVals are slightly more tricky than any other value in the symbolic domain. We probably just messed it up. I could look into this.

[NagyDonat]

I'm reopening this issue manually because #163461 mentions that it "Reopens" this, but that apparently isn't automatically recognized by github.