jeremyncc opened a new issue #1360: URL: https://github.com/apache/incubator-nuttx/issues/1360
### 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](https://www.fsl.cs.sunysb.edu/kernel-api/re257.html ) 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. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
