Many IOCTL handlers do not validate the address of their argument

[jeremyncc]

Location:

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

Impact:

A malicious user space application can corrupt kernel memory by passing pointers to IOCTL arguments that overlap the kernel memory space.

Description:

NuttX kernel driver IOCTLs receive arguments from user space in an opaque unsigned int type. This arg value is then cast to a pointer, and the kernel driver will either write to this pointer (to return data to user space) or read from this pointer (to receive data from user space).

Throughout the NuttX kernel, this pointer is not validated prior to use. A compromised user-space application may pass a pointer in arg that points to kernel memory rather than user memory. If the kernel does not validate the address stored in arg, then the kernel could be coerced into overwriting its own memory when attempting to return data to userspace.

One such example is shown below for the SmartFS file system. The BIOC_GETPROCFSD is a block device IOCTL that is handled by the smart_ioctl function in smart.c:

static int smart_ioctl(FAR struct inode *inode, int cmd, unsigned long arg)
{
    ...
    case BIOC_GETPROCFSD:
      /* Get ProcFS data */
      procfs_data = (FAR struct mtd_smart_procfs_data_s *) arg;
      procfs_data->totalsectors = dev->total_physsectors;
      procfs_data->sectorsize = dev->sectorsize;
      procfs_data->freesectors = dev->free_physsectors;
      procfs_data->releasesectors = dev->release_physsectors;
      procfs_data->namelen = dev->namesize;
      procfs_data->formatversion = dev->formatversion;
      procfs_data->unusedsectors = dev->unusedsectors;
      procfs_data->blockerases = dev->blockerases;
      procfs_data->sectorsperblk = dev->sectorsPerBlk;
    ...

Above, the arg value is cast to a pointer, and aliased to procfs_data structure. The structure members are then initialized, so that the procfs device configuration can be returned to user-space. However, because a malicious user-space application could pass an arg pointer that overlaps the kernel's address space, this structure initialization could result in kernel memory corruption.

Almost all other IOCTLs in the SmartFS block device driver exhibit similar behavior. Furthermore, although NCC Group could not review the entire NuttX kernel, a very quick analysis shows that this problem is pervasive and most, if not all, IOCTLs lack argument validation to ensure the passed-in pointer actuall points to userspace memory.

Although discovered independently, this appears to be a duplicate of this mailing list message.

Recommendation:

The recommendation action is to borrow a solution from other operating systems. For example, in Linux, the access_ok function is used in IOCTLs to ensure that a passed-in pointer does not overlap the kernel memory space. NuttX should introduce such a semantic to allow the kernel to protect itself from malicious user-space applications.

[patacongo]

This is pretty similar to Issue #1329

[jeremyncc]

Ah, it is indeed. My apologies. I did not review the existing list of issues before filing this one.

[patacongo]

Ah, it is indeed. My apologies. I did not review the existing list of issues before filing this one.

Yes, it is the same root issue: This PR specifically addresses ioctls and the other only specifies read(), but I think it generalizes into any system call that receives a write-able pointer.

There are several other Issues related to the PROTECTED mode that I have opened. You can see that they are all tagged with the Security label.

I have also taken some effort to obfuscate the stack content on some call backs from the OS -- signal handlers, atexit(), on_exit(), pthread_cleanup functions, pthread-specific data destructors, etc. But that is incomplete and insufficient and also deserves to have a new Issue opened.

Unlike Linux, there are not separate stacks for user logic and system functions so callbacks from the OS expose the entire stack and, since it is user-writable, is also a glaring security hole since a malicious app can modify the stack content before returning.

[protobits]

I will close this since the general case is reported in #1329 and this issue is linked from there for reference.