The `aac_ioctl_send_raw_srb` function can be reached by supplying the `FSACTL_LNX_SEND_RAW_SRB` command to `aac_ioctl`. This code path dereferences a user supplied pointer directly: static int aac_ioctl_send_raw_srb(struct aac_softc *sc, caddr_t arg) { struct aac_srb *user_srb = (struct aac_srb *)arg; ... if ((error = copyin((void *)user_srb, srbcmd, fibsize) != 0)) goto out; ... struct aac_sg_entry *sgp = srbcmd->sg_map.SgEntry; ... srb_sg_bytecount = sgp->SgByteCount; ... } `srbcmd` has user controlled contents (after `copyin` from `user_srb`). `sgp` is set to a user controlled address (`srbcmd->sg_map.SgEntry`). `sgp` is then dereferenced numerous times (`sgp->SgByteCount`). One impact of this improper handling is that `sgp` could be `NULL`, which would result in a `NULL` dereference, and panic.
I've committed a patch to HardenedBSD: https://github.com/HardenedBSD/hardenedBSD-playground/commit/48d6f11271b93a265184de813e32dba8f5cf76f9
This bug is also present in the `aac` (not aacraid) code in the same function: here: /* Retrieve correct SG entries. */ if (fibsize == (sizeof(struct aac_srb) + srbcmd->sg_map.SgCount * sizeof(struct aac_sg_entry))) { sge = srbcmd->sg_map.SgEntry; sge64 = NULL; srb_sg_bytecount = sge->SgByteCount; srb_sg_address = (void *)(uintptr_t)sge->SgAddress; } and here: https://github.com/freebsd/freebsd/blob/bac8688b17d735d252ec75a94df67384938f3f9b/sys/dev/aac/aac.c#L3114-L3122 #ifdef __amd64__ else if (fibsize == (sizeof(struct aac_srb) + srbcmd->sg_map.SgCount * sizeof(struct aac_sg_entry64))) { sge = NULL; sge64 = (struct aac_sg_entry64 *)srbcmd->sg_map.SgEntry; srb_sg_bytecount = sge64->SgByteCount; ... } #endif
Removed the link for the first chunk by accident: https://github.com/freebsd/freebsd/blob/bac8688b17d735d252ec75a94df67384938f3f9b/sys/dev/aac/aac.c#L3104-L3110
Any movement on this?
Any update with this issue?!
Cturt: have you finished the exploit for this issue?
Created attachment 169475 [details] unified patch for aac and aacraid Ok, this is what I came up with for aac that duplicates the behavior of aacraid patch from cturt. Comments?
(In reply to Sean Bruno from comment #7) This patch looks good to me. Thanks, Sean.
A commit references this bug: Author: sbruno Date: Tue Apr 19 18:27:29 UTC 2016 New revision: 298280 URL: https://svnweb.freebsd.org/changeset/base/298280 Log: aacraid(4): Sanely copyin userland pointers and ensure that we don't get anything janky from a user. (cturt) aac(4): landergriffith+freebsdbugzilla@gmail.com pointed out that aacraid(4) had the same issue and handling of pointers, so let's change that too. PR: 206573 Submitted by: cturt@hardenedbsd.org Obtained from: HardenedBSD MFC after: 1 week Changes: head/sys/dev/aac/aac.c head/sys/dev/aacraid/aacraid.c
A commit references this bug: Author: sbruno Date: Fri May 6 19:18:45 UTC 2016 New revision: 299193 URL: https://svnweb.freebsd.org/changeset/base/299193 Log: MFC r298280 aacraid(4): Sanely copyin userland pointers and ensure that we don't get anything janky from a user. (cturt) aac(4) landergriffith+freebsdbugzilla@gmail.com pointed out that aac(4) had the same issue and handling of pointers, so let's change that too. PR: 206573 Submitted by: cturt@hardenedbsd.org Changes: _U stable/10/ stable/10/sys/dev/aac/aac.c stable/10/sys/dev/aacraid/aacraid.c