|
| 1 | +/** |
| 2 | + * @id c/cert/appropriate-thread-object-storage-durations |
| 3 | + * @name CON34-C: Declare objects shared between threads with appropriate storage durations |
| 4 | + * @description Accessing thread-local variables with automatic storage durations can lead to |
| 5 | + * unpredictable program behavior. |
| 6 | + * @kind problem |
| 7 | + * @precision high |
| 8 | + * @problem.severity error |
| 9 | + * @tags external/cert/id/con34-c |
| 10 | + * correctness |
| 11 | + * concurrency |
| 12 | + * external/cert/obligation/rule |
| 13 | + */ |
| 14 | + |
| 15 | +import cpp |
| 16 | +import codingstandards.c.cert |
| 17 | +import codingstandards.cpp.Concurrency |
| 18 | +import semmle.code.cpp.dataflow.TaintTracking |
| 19 | + |
| 20 | + |
| 21 | +/// anything from tss_get is ok. the tss get must obtain the value from the |
| 22 | +// context that called tss_set NOT in the thread. |
| 23 | +/// anything that was static is ok |
| 24 | +/// anything dynamically allocated is ok. |
| 25 | + |
| 26 | +// THREAD LOCAL IS NOT OK -- EG STACK VARIBLES ARE NEVER OK |
| 27 | + |
| 28 | +// we can make this really simple -- just look for thread create functions |
| 29 | +// wherein you pass in a variable created on the stack that is a) not static or |
| 30 | +// b) not created via malloc and c) not obtained from tss_get. |
| 31 | +// we should do something more to determine if tss_get is wrongly called from a |
| 32 | +// thread context without a matching tss_get as another query. That should be an |
| 33 | +// audit. tss get without set. |
| 34 | +// tss_get in the parent thread should maybe be followed by a thread_join |
| 35 | +// function |
| 36 | +// |
| 37 | +// |
| 38 | + |
| 39 | +// IN THE PARENT -- a call to tss_set MUST be followed by a thread_join. |
| 40 | +// Without this, it is possible the context isn't valid anymore. |
| 41 | + |
| 42 | +// It's important to note -- tss_get set DOES NOT require a call to delete |
| 43 | +// to require the wait. It just requires the wait if it is used at all. |
| 44 | + |
| 45 | +class MallocFunctionCall extends FunctionCall { |
| 46 | + MallocFunctionCall(){ |
| 47 | + getTarget().getName() = "malloc" |
| 48 | + } |
| 49 | +} |
| 50 | + |
| 51 | +from MallocFunctionCall fc, StackVariable sv, Expr e |
| 52 | +where not isExcluded(fc) |
| 53 | +and TaintTracking::localTaint(DataFlow::exprNode(fc), DataFlow::exprNode(e)) |
| 54 | +select fc, e |
| 55 | + |
| 56 | + |
| 57 | +// from C11ThreadCreateCall tcc, StackVariable sv, Expr arg |
| 58 | +// where |
| 59 | +// not isExcluded(tcc, Concurrency4Package::appropriateThreadObjectStorageDurationsQuery()) and |
| 60 | +// tcc.getArgument(2) = arg and |
| 61 | +// // a stack variable that is given as an argument to a thread |
| 62 | +// TaintTracking::localTaint(DataFlow::exprNode(sv.getAnAccess()), DataFlow::exprNode(arg)) |
| 63 | +// // that isn't one of the allowed usage patterns |
| 64 | +// TaintTracking::localTaint(DataFlow::exprNode(sv.getAnAccess()), DataFlow::exprNode(arg)) |
| 65 | +// select tcc, "$@ not declared with appropriate storage duration", arg, "Shared object" |
0 commit comments