analyzer: look through casts in taint sanitization [PR112974,PR112975]

PR analyzer/112974 and PR analyzer/112975 record false positives
from the analyzer's taint detection where sanitization of the form

  if (VALUE CMP VALUE-OF-WIDER-TYPE)

happens, but wasn't being "noticed" by the taint checker, due to the
test being:

  (WIDER_TYPE)VALUE CMP VALUE-OF-WIDER-TYPE

at the gimple level, and thus taint_state_machine recording
sanitization of (WIDER_TYPE)VALUE, but not of VALUE.

Fix by stripping casts in taint_state_machine::on_condition so that
the state machine records sanitization of the underlying value.

gcc/analyzer/ChangeLog:
	PR analyzer/112974
	PR analyzer/112975
	* sm-taint.cc (taint_state_machine::on_condition): Strip away
	casts before considering LHS and RHS, to increase the chance of
	detecting places where sanitization of a value may have happened.

gcc/testsuite/ChangeLog:
	PR analyzer/112974
	PR analyzer/112975
	* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
	taint-pr112974.c and taint-pr112975.c to analyzer_kernel_plugin.c.
	* gcc.dg/plugin/taint-pr112974.c: New test.
	* gcc.dg/plugin/taint-pr112975.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index c873c9ebd3337d2db47d6fdc418e6ac08e41edfe..1d1e208fdf49bb29509da025490fbf74e8046421 100644 (file)
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -1109,6 +1109,14 @@ taint_state_machine::on_condition (sm_context *sm_ctxt,
       return;
     }
 
+  /* Strip away casts before considering LHS and RHS, to increase the
+     chance of detecting places where sanitization of a value may have
+     happened.  */
+  if (const svalue *inner = lhs->maybe_undo_cast ())
+    lhs = inner;
+  if (const svalue *inner = rhs->maybe_undo_cast ())
+    rhs = inner;
+
   // TODO
   switch (op)
     {

diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index c26dda1f324baa0b6f302f532b008fd04165f12f..933f9a5850bc238c392622cb260a707ac8457d67 100644 (file)
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -172,6 +172,8 @@ set plugin_test_list [list \
          taint-pr112850-too-complex.c \
          taint-pr112850-unsanitized.c \
          taint-pr112927.c \
+         taint-pr112974.c \
+         taint-pr112975.c \
          taint-pr112977.c } \
     { analyzer_cpython_plugin.c \
          cpython-plugin-test-no-Python-h.c \

diff --git a/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c b/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c
new file mode 100644 (file)
index 0000000..1af5053
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c
@@ -0,0 +1,59 @@
+/* Reduced from false positive in Linux kernel in
+   drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c.  */
+
+/* { dg-do compile } */
+/* { dg-options "-fanalyzer" } */
+/* { dg-require-effective-target analyzer } */
+
+typedef unsigned char __u8;
+typedef unsigned short __u16;
+extern unsigned int __max_logical_packages;
+extern unsigned long
+copy_from_user(void* to, const void* from, unsigned long n);
+extern unsigned long
+copy_to_user(void* to, const void* from, unsigned long n);
+struct isst_tpmi_instance_count
+{
+  __u8 socket_id;
+  __u8 count;
+  __u16 valid_mask;
+};
+struct tpmi_per_power_domain_info
+{
+  void* sst_base;
+};
+struct tpmi_sst_struct
+{
+  int number_of_power_domains;
+  struct tpmi_per_power_domain_info* power_domain_info;
+};
+struct tpmi_sst_common_struct
+{
+  int max_index;
+  struct tpmi_sst_struct** sst_inst;
+};
+static struct tpmi_sst_common_struct isst_common;
+int
+isst_if_get_tpmi_instance_count(void* argp)
+{
+  struct isst_tpmi_instance_count tpmi_inst;
+  struct tpmi_sst_struct* sst_inst;
+  int i;
+  if (copy_from_user(&tpmi_inst, argp, sizeof(tpmi_inst)))
+    return -14;
+  if (tpmi_inst.socket_id >= (__max_logical_packages))
+    return -22;
+  tpmi_inst.count =
+    isst_common.sst_inst[tpmi_inst.socket_id]->number_of_power_domains; /* { dg-bogus "use of attacker-controlled value as offset without upper-bounds checking" } */
+  sst_inst = isst_common.sst_inst[tpmi_inst.socket_id];
+  tpmi_inst.valid_mask = 0;
+  for (i = 0; i < sst_inst->number_of_power_domains; ++i) {
+    struct tpmi_per_power_domain_info* pd_info;
+    pd_info = &sst_inst->power_domain_info[i];
+    if (pd_info->sst_base)
+      tpmi_inst.valid_mask |= ((((1UL))) << (i));
+  }
+  if (copy_to_user(argp, &tpmi_inst, sizeof(tpmi_inst)))
+    return -14;
+  return 0;
+}

diff --git a/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c b/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c
new file mode 100644 (file)
index 0000000..9f312cb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c
@@ -0,0 +1,53 @@
+/* Reduced from false positive in Linux kernel in
+   drivers/xen/privcmd.c.  */
+
+/* { dg-do compile } */
+/* { dg-options "-fanalyzer" } */
+/* { dg-require-effective-target analyzer } */
+
+typedef __SIZE_TYPE__ size_t;
+typedef unsigned short __u16;
+typedef unsigned int gfp_t;
+void
+kfree(const void* objp);
+
+extern void *
+__attribute__((__alloc_size__(1, 2)))
+__attribute__((__malloc__))
+kcalloc(size_t n, size_t size, gfp_t flags);
+
+extern unsigned long
+copy_from_user(void*, const void*, unsigned long);
+
+typedef __u16 domid_t;
+struct privcmd_dm_op_buf
+{
+  void* uptr;
+  size_t size;
+};
+struct privcmd_dm_op
+{
+  domid_t dom;
+  __u16 num;
+};
+static unsigned int privcmd_dm_op_max_num = 16;
+long
+privcmd_ioctl_dm_op(void* udata)
+{
+  struct privcmd_dm_op kdata;
+  struct privcmd_dm_op_buf* kbufs;
+  if (copy_from_user(&kdata, udata, sizeof(kdata)))
+    return -14;
+  if (kdata.num == 0)
+    return 0;
+  if (kdata.num > privcmd_dm_op_max_num)
+    return -7;
+  kbufs =
+    kcalloc(kdata.num,  /* { dg-bogus "attacker-controlled value" }  */
+            sizeof(*kbufs),
+            (((gfp_t)(0x400u | 0x800u)) | ((gfp_t)0x40u) | ((gfp_t)0x80u)));
+  if (!kbufs)
+    return -12;
+  kfree(kbufs);
+  return 0;
+}