[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

Thu Feb 23 02:04:53 PST 2023

dkrupp added a comment.

@steakhal, @NoQ

thanks for your reviews.

Please note that I am not extending `TaintBugVisitor`. On the contrary I removed it.
Instead I use NoteTag to generate the "Taint Originated here" text (see GenericTaintChecker.cpp:156).

I can also add additional NoteTags for generating propagation messages "Taint was propagated here" easily.

> The challenging part with note tags is how do you figure out whether your bug report is taint-related.

I solved this by checking if the report is an instance of `TaintBugReport` a new BugReport type, which should be used by all taint related reports (ArrayBoundCheckerV2 checker and divisionbyzero checker 
was changed to use this new report type for taint related reports).

The tricky part was is to how to show only that single "Taint originated here" note tag at the taint source only which is relevant to the report. This is done by remembering the unique flowid in the
NoteTag in the forward analysis direction (see GenericTaintChecker:cpp:859) `InjectionTag = createTaintOriginTag(C, TaintFlowId);` and then filtering out the irrelevant 
NoteTags when the the report is generated (when the lamdba callback is called). See that flowid which reaches the sink is backpropagated in the PathSensitiveBugreport (see GenericTaintCHekcer.cpp:167).

FlowIds are unique and increased at every taint source (GenericTaintChecker:869) and it is stored as an additional simple `int` in the program state along with the already existing (Taint.cpp:22) TaintTagType.

My fear with the interestingness is that it may propagating backwards according to different "rules" than whot the taintedness is popagating in the foward direction even with the "extensions" pointed out by steakhal.
So the two types of propagation may be or can get out of sync.

So if the above is not a concern and you think implementing this with interestingness is more elegant, idiomatic and causes less maintenance burden, I am happy to create an alternative patch with that solution.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144269/new/

https://reviews.llvm.org/D144269