You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/07/03 18:49:53 UTC
[GitHub] [incubator-nuttx] jeremyncc opened a new issue #1359: Many IOCTL handlers do not validate the address of their argument
jeremyncc opened a new issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359
### 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](https://www.mail-archive.com/dev@nuttx.apache.org/msg02337.html).
### Recommendation:
The recommendation action is to borrow a solution from other operating systems. For example, in Linux, the [access_ok](https://www.fsl.cs.sunysb.edu/kernel-api/re243.html) 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.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359#issuecomment-653672158
>
>
> 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.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
v01d commented on issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359#issuecomment-810457008
I will close this since the general case is reported in #1329 and this issue is linked from there for reference.
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] jeremyncc commented on issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
jeremyncc commented on issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359#issuecomment-653669811
Ah, it is indeed. My apologies. I did not review the existing list of issues before filing this one.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d closed issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
v01d closed issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359#issuecomment-653665637
This is pretty similar to Issue #1329
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #1359: Many IOCTL handlers do not validate the address of their argument
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #1359:
URL: https://github.com/apache/incubator-nuttx/issues/1359#issuecomment-653672158
>
>
> 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 issues: 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. That 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.
----------------------------------------------------------------
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:
users@infra.apache.org