Implement rule SIG35-C

Author: mbaluda

c/cert/src/rules/SIG30-C/CallOnlyAsyncSafeFunctionsWithinSignalHandlers.ql

@@ -17,16 +17,15 @@ import codingstandards.c.Signal
 import semmle.code.cpp.dataflow.DataFlow
 
 /**
- * Does not an access an external variable except
- * to assign a value to a volatile static variable of sig_atomic_t type
+ * Does not access an external variable except
+ * to assign a value to a volatile static variable of `sig_atomic_t` type
  */
 class AsyncSafeVariableAccess extends VariableAccess {
   AsyncSafeVariableAccess() {
     this.getTarget() instanceof StackVariable
     or
-    this.getType().hasName("volatile sig_atomic_t") and // TODO search without "volatile"
-    this.isModified() and
-    this.getTarget().isVolatile()
+    this.getTarget().(StaticStorageDurationVariable).getType().(SigAtomicType).isVolatile() and
+    this.isModified()
   }
 }
 

c/cert/src/rules/SIG31-C/DoNotAccessSharedObjectsInSignalHandlers.ql

@@ -27,10 +27,8 @@ class UnsafeSharedVariableAccess extends VariableAccess {
       this.getTarget().isThreadLocal()
     ) and
     // excluding `volatile sig_atomic_t` type
-    not (
-      this.getType().hasName("volatile sig_atomic_t") and // TODO search without "volatile"
-      this.getTarget().isVolatile()
-    ) and //excluding lock-free atomic objects
+    not this.getType().(SigAtomicType).isVolatile() and
+    // excluding lock-free atomic objects
     not exists(MacroInvocation mi, VariableAccess va |
       mi.getMacroName() = "atomic_is_lock_free" and
       mi.getExpr().getChild(0) = va.getEnclosingElement*() and

c/cert/src/rules/SIG34-C/DoNotCallSignalFromInterruptibleSignalHandlers.ql

@@ -6,16 +6,19 @@
  * @precision very-high
  * @problem.severity error
  * @tags external/cert/id/sig34-c
+ *       correctness
+ *       security
  *       external/cert/obligation/rule
  */
 
 import cpp
 import codingstandards.c.cert
 import codingstandards.c.Signal
 
-from FunctionCall x
+from FunctionCall signal
 where
-  not isExcluded(x, SignalHandlersPackage::doNotCallSignalFromInterruptibleSignalHandlersQuery()) and
-  x = any(SignalHandler handler).getReassertingCall()
-select x,
+  not isExcluded(signal,
+    SignalHandlersPackage::doNotCallSignalFromInterruptibleSignalHandlersQuery()) and
+  signal = any(SignalHandler handler).getReassertingCall()
+select signal,
   "Reasserting handler bindings introduces a race condition on nonpersistent platforms and is redundant otherwise."

c/cert/src/rules/SIG35-C/DoNotReturnFromAComputationalExceptionHandler.ql

@@ -6,13 +6,38 @@
  * @precision very-high
  * @problem.severity error
  * @tags external/cert/id/sig35-c
+ *       correctness
+ *       security
  *       external/cert/obligation/rule
  */
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.c.Signal
+import semmle.code.cpp.dataflow.DataFlow
 
-from
+/**
+ * CFG nodes preceeding a `ReturnStmt`
+ */
+ControlFlowNode reachesReturn(ReturnStmt return) {
+  result = return
+  or
+  exists(ControlFlowNode mid |
+    result = mid.getAPredecessor() and
+    mid = reachesReturn(return) and
+    // stop recursion on calls to `abort`, `_Exit` and "quick_exit"
+    not result instanceof AbortCall
+  )
+}
+
+from ReturnStmt return, ComputationalExceptionSignal e
 where
-  not isExcluded(x, SignalHandlersPackage::doNotReturnFromAComputationalExceptionHandlerQuery()) and
-select
+  not isExcluded(return, SignalHandlersPackage::doNotReturnFromAComputationalExceptionHandlerQuery()) and
+  exists(SignalHandler handler |
+    handler = return.getEnclosingFunction() and
+    // computational exception handler
+    DataFlow::localExprFlow(e.getExpr(), handler.getRegistration().getArgument(0)) and
+    // control flow reaches a return statement
+    reachesReturn(return) = handler.getBlock()
+  )
+select return, "Do not return from a $@ signal handler.", e, "computational exception"

c/cert/test/rules/SIG35-C/DoNotReturnFromAComputationalExceptionHandler.expected

@@ -1 +1 @@
-No expected results have yet been specified
+| test.c:10:1:10:1 | return ... | Do not return from a $@ signal handler. | test.c:13:10:13:15 | SIGFPE | computational exception |

c/cert/test/rules/SIG35-C/test.c

@@ -0,0 +1,26 @@
+#include <errno.h>
+#include <limits.h>
+#include <signal.h>
+#include <stdlib.h>
+
+volatile sig_atomic_t eflag;
+
+void sighandle(int s) { // NON_COMPLIANT
+  eflag = 1;
+}
+
+int f1(int argc, char *argv[]) {
+  signal(SIGFPE, sighandle);
+
+  return 0;
+}
+
+void sighandle2(int s) { // COMPLIANT
+  eflag = 1;
+  abort();
+}
+
+int f2(int argc, char *argv[]) {
+  signal(SIGFPE, sighandle2);
+  return 0;
+}

c/common/src/codingstandards/c/Signal.qll

@@ -1,6 +1,13 @@
 import cpp
 import semmle.code.cpp.dataflow.DataFlow
 
+/**
+ * A signal corresponding to a computational exception
+ */
+class ComputationalExceptionSignal extends MacroInvocation {
+  ComputationalExceptionSignal() { this.getMacroName() = ["SIGFPE", "SIGILL", "SIGSEGV", "SIGBUS"] }
+}
+
 /**
  * A call to function `signal`
  */
@@ -39,3 +46,16 @@ class SignalHandler extends Function {
 class AbortCall extends FunctionCall {
   AbortCall() { this.getTarget().hasGlobalName(["abort", "_Exit", "quick_exit"]) }
 }
+
+/**
+ * Models the type `sig_atomic_type`
+ */
+class SigAtomicType extends Type {
+  SigAtomicType() {
+    this.getName() = "sig_atomic_t"
+    or
+    this.(TypedefType).getBaseType() instanceof SigAtomicType
+    or
+    this.(SpecifiedType).getBaseType() instanceof SigAtomicType
+  }
+}

rule_packages/c/SignalHandlers.json

@@ -55,7 +55,10 @@
           "precision": "very-high",
           "severity": "error",
           "short_name": "DoNotCallSignalFromInterruptibleSignalHandlers",
-          "tags": []
+          "tags": [
+            "correctness",
+            "security"
+          ]
         }
       ],
       "title": "Do not call signal() from within interruptible signal handlers"
@@ -72,7 +75,10 @@
           "precision": "very-high",
           "severity": "error",
           "short_name": "DoNotReturnFromAComputationalExceptionHandler",
-          "tags": []
+          "tags": [
+            "correctness",
+            "security"
+          ]
         }
       ],
       "title": "Do not return from a computational exception signal handler"