From a4fc7650ee0077ca61b6366dfe0aafb966da9ebb Mon Sep 17 00:00:00 2001
From: Ben Toews <mastahyeti@gmail.com>
Date: Tue, 19 Jun 2018 12:49:48 -0600
Subject: [PATCH 1/2] fix null pointer dereference in driver userclient

IOMemoryDescriptor.map() will return NULL if the length is 0. This can
be triggered by sending a report with a single null byte.
---
 .../xcshareddata/IDEWorkspaceChecks.plist          |  8 ++++++++
 SoftU2FDriver/SoftU2FUserClient.cpp                | 14 +++++++-------
 script/run                                         |  4 ++--
 3 files changed, 17 insertions(+), 9 deletions(-)
 create mode 100644 SoftU2F.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist

diff --git a/SoftU2F.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist b/SoftU2F.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
new file mode 100644
index 0000000..18d9810
--- /dev/null
+++ b/SoftU2F.xcodeproj/project.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>IDEDidComputeMac32BitWarning</key>
+	<true/>
+</dict>
+</plist>
diff --git a/SoftU2FDriver/SoftU2FUserClient.cpp b/SoftU2FDriver/SoftU2FUserClient.cpp
index 1fe6196..0fe8b7b 100644
--- a/SoftU2FDriver/SoftU2FUserClient.cpp
+++ b/SoftU2FDriver/SoftU2FUserClient.cpp
@@ -156,21 +156,21 @@ void SoftU2FUserClient::frameReceived(IOMemoryDescriptor *report) {
 void SoftU2FUserClient::frameReceivedGated(IOMemoryDescriptor *report) {
   IOLog("%s[%p]::%s(%p)\n", getName(), this, __FUNCTION__, report);
 
-  IOMemoryMap *reportMap = nullptr;
+  IOMemoryMap *reportMap;
+  io_user_reference_t *args;
 
-  if (isInactive())
+  if (isInactive() || !_notifyRef)
     return;
 
-  if (report->prepare() != kIOReturnSuccess)
+  if (report->getLength() != sizeof(U2FHID_FRAME) || report->prepare() != kIOReturnSuccess)
     return;
 
+  // Map report into kernel space.
   reportMap = report->map();
 
   // Notify userland that we got a report.
-  if (_notifyRef && reportMap->getLength() == sizeof(U2FHID_FRAME)) {
-    io_user_reference_t *args = (io_user_reference_t *)reportMap->getAddress();
-    sendAsyncResult64(*_notifyRef, kIOReturnSuccess, args, sizeof(U2FHID_FRAME) / sizeof(io_user_reference_t));
-  }
+  args = (io_user_reference_t *)reportMap->getAddress();
+  sendAsyncResult64(*_notifyRef, kIOReturnSuccess, args, sizeof(U2FHID_FRAME) / sizeof(io_user_reference_t));
 
   reportMap->release();
   report->complete();
diff --git a/script/run b/script/run
index 2ee31f2..7e09bea 100755
--- a/script/run
+++ b/script/run
@@ -20,10 +20,10 @@ if kextstat -b $BUNDLE_ID | grep $BUNDLE_ID &> /dev/null; then
 fi
 
 # Ensure kext is owned by root.
-sudo chown -R root:wheel $KEXT_PATH
+sudo chown -R root:wheel "${KEXT_PATH}"
 
 echo "Loading softu2f.kext"
-if ! sudo kextutil $KEXT_PATH; then
+if ! sudo kextutil "${KEXT_PATH}"; then
   echo "Error loading softu2f.kext"
   exit 1
 fi

From 9f4ff586808bfe97cd80fdad1856985f0a56c5a0 Mon Sep 17 00:00:00 2001
From: Ben Toews <mastahyeti@gmail.com>
Date: Tue, 19 Jun 2018 13:01:24 -0600
Subject: [PATCH 2/2] may as well check return value from report->map() too

---
 SoftU2FDriver/SoftU2FUserClient.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/SoftU2FDriver/SoftU2FUserClient.cpp b/SoftU2FDriver/SoftU2FUserClient.cpp
index 0fe8b7b..a006d29 100644
--- a/SoftU2FDriver/SoftU2FUserClient.cpp
+++ b/SoftU2FDriver/SoftU2FUserClient.cpp
@@ -156,8 +156,7 @@ void SoftU2FUserClient::frameReceived(IOMemoryDescriptor *report) {
 void SoftU2FUserClient::frameReceivedGated(IOMemoryDescriptor *report) {
   IOLog("%s[%p]::%s(%p)\n", getName(), this, __FUNCTION__, report);
 
-  IOMemoryMap *reportMap;
-  io_user_reference_t *args;
+  IOMemoryMap *reportMap = nullptr;
 
   if (isInactive() || !_notifyRef)
     return;
@@ -168,11 +167,13 @@ void SoftU2FUserClient::frameReceivedGated(IOMemoryDescriptor *report) {
   // Map report into kernel space.
   reportMap = report->map();
 
-  // Notify userland that we got a report.
-  args = (io_user_reference_t *)reportMap->getAddress();
-  sendAsyncResult64(*_notifyRef, kIOReturnSuccess, args, sizeof(U2FHID_FRAME) / sizeof(io_user_reference_t));
+  if (reportMap != nullptr) {
+    // Notify userland that we got a report.
+    io_user_reference_t *args = (io_user_reference_t *)reportMap->getAddress();
+    sendAsyncResult64(*_notifyRef, kIOReturnSuccess, args, sizeof(U2FHID_FRAME) / sizeof(io_user_reference_t));
+    reportMap->release();
+  }
 
-  reportMap->release();
   report->complete();
 }