Arguments passed to IOCTL handlers may be vulnerable to time-of-check-time-of-use race conditions

[jeremyncc]

Location:

https://github.com/apache/incubator-nuttx/blob/master/drivers/mtd/smart.c#L4931

Impact:

In architectures that support multi-processing, a malicious user space application can modify arguments passed to a kernel syscall or IOCTL handler while the kernel is in the midst of operating on those arguments. This may lead to race conditions that can result in bypassing input validation and can lead to kernel memory corruption.

Description:

On CPU architectures that support multi-processing, the kernel and user space may execute in parallel. In this situation, the kernel must be careful to defend itself from race conditions related to memory pointers that are passed as syscall arguments. Kernels typically defend against these types of race conditions by performing a deep copy of the IOCTL arguments immediately upon entering the IOCTL handler. Failure to make a local copy of the arguments means that the kernel is operating on memory that is controlled by a potentially malicious userspace.

This particular class of race condition is referred to as a time-of-check-time-of-use (TOCTOU) vulnerability. Throughout the NuttX kernel, NCC Group noticed that IOCTL handlers do not make a local copy of the arguments. Though, NCC Group did not review the entire NuttX kernel to confirm this.

One such example is shown in smart.c for the BIOC_READSECT IOCTL handler named smart_readsector, shown below:

static int smart_readsector(FAR struct smart_struct_s *dev,
                    unsigned long arg)
{
  int       ret;
  uint16_t  physsector;
  FAR struct smart_read_write_s *req;
#ifdef CONFIG_MTD_SMART_ENABLE_CRC
#if SMART_STATUS_VERSION == 1
  FAR struct smart_sect_header_s *header;
#endif
#else
  uint32_t  readaddr;
  struct smart_sect_header_s header;
#endif

  finfo("Entry\n");

  req = (FAR struct smart_read_write_s *) arg;
  DEBUGASSERT(req->offset < dev->sectorsize);
  DEBUGASSERT(req->offset + req->count + sizeof(struct smart_sect_header_s)
              <= dev->sectorsize);

  /* Ensure the logical sector has been allocated */

  if (req->logsector >= dev->totalsectors)
    {
      ferr("ERROR: Logical sector %d too large\n", req->logsector);

      ret = -EINVAL;
      goto errout;
    }
#ifndef CONFIG_MTD_SMART_MINIMIZE_RAM
  physsector = dev->smap[req->logsector];
#else
  physsector = smart_cache_lookup(dev, req->logsector);
#endif
  ...

In the above code, req points to a structure that resides in userspace memory. The req structure members are validated using a DEBUGASSERT and a conditional check in an if statement. However, userspace could modify the contents of req after these validation steps are performed. In particular, an attacker could modify req->logsector to be very large, which has the effect of calculating an out-of-bounds index into dev->smap[].

Although NCC Group did not review the entirety of the NuttX kernel, it is suspected that other IOCTL handlers contain similar weaknesses. Typically, TOCTOU problems can allow a malicious userspace application to bypass input validation which can result in kernel memory corrupt or memory exposure.

Recommendation:

NCC Group did not discover any evidence of a copy_from_user style of function in the NuttX kernel. This function is used in the Linux kernel in order to make a local deep copy of the IOCTL arguments, preventing TOCTOU-style vulnerabilities. An equivalent copy_to_user performs the reverse operation. NuttX should introduce such a semantic to allow the kernel to protect itself from a malicious userspace.