[PATCH] ios: Avoid data leak and write-beyond-bounds in mem.

[Eric Blake]

We were leaking 4096 bytes of the previous contents of the heap if you
read from a memory buffer without a prior write.  What's more, if you
attempted to write beyond the current bound of the buffer, we silently
allowed ANY offset to be set, but then would only grow the array by
4096 bytes and not update the size (as visible through iosize(0)),
meaning we gave a user an arbitrary write-anywhere gadget (even though
they could not then read at those arbitrary offsets).  Let's clamp
things down to ensure that memory buffers always start life with known
contents, and that while we still permit memory buffers to auto-grow,
we only do so 4096 bytes at a time.

* src/ios-dev-mem.c (ios_dev_mem_putc): Zero the added bytes, and
track the updated size.
(ios_dev_mem_seek): Forbid seeks beyond our desired growth amount.
(ios_dev_mem_open): Zero the initial bytes.
* testsuite/poke.cmd/mem-1.pk: New test.
* testsuite/poke.cmd/mem-2.pk: Likewise.
* testsuite/poke.cmd/mem-3.pk: Likewise.
---

This is a fix for the security hole I identified last week (I don't
think a CVE is needed since we haven't released poke, or I would not
have been so public about the issue last week).

 ChangeLog                   | 11 +++++++++++
 src/ios-dev-mem.c           | 22 ++++++++++++++++------
 testsuite/poke.cmd/mem-1.pk |  6 ++++++
 testsuite/poke.cmd/mem-2.pk | 11 +++++++++++
 testsuite/poke.cmd/mem-3.pk |  6 ++++++
 5 files changed, 50 insertions(+), 6 deletions(-)
 create mode 100644 testsuite/poke.cmd/mem-1.pk
 create mode 100644 testsuite/poke.cmd/mem-2.pk
 create mode 100644 testsuite/poke.cmd/mem-3.pk

diff --git a/ChangeLog b/ChangeLog
index 75fd2541..1dcf148a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2020-03-02  Eric Blake  <address@hidden>
+
+       ios: Avoid data leak and write-beyond-bounds in mem.
+       * src/ios-dev-mem.c (ios_dev_mem_putc): Zero the added bytes, and
+       track the updated size.
+       (ios_dev_mem_seek): Forbid seeks beyond our desired growth amount.
+       (ios_dev_mem_open): Zero the initial bytes.
+       * testsuite/poke.cmd/mem-1.pk: New test.
+       * testsuite/poke.cmd/mem-2.pk: Likewise.
+       * testsuite/poke.cmd/mem-3.pk: Likewise.
+
 2020-03-02  Eric Blake  <address@hidden>

        * .gitignore: Broaden exclusion to cover all build objects.
diff --git a/src/ios-dev-mem.c b/src/ios-dev-mem.c
index 72b5e9a1..a0d6e107 100644
--- a/src/ios-dev-mem.c
+++ b/src/ios-dev-mem.c
@@ -17,6 +17,8 @@
  */

 #include <config.h>
+
+#include <assert.h>
 #include <string.h>
 #include <stdlib.h>
 #include <xalloc.h>
@@ -50,7 +52,7 @@ ios_dev_mem_open (const char *handler, uint64_t flags, int 
*error)
   struct ios_dev_mem *mio;

   mio = xmalloc (sizeof (struct ios_dev_mem));
-  mio->pointer = xmalloc (MEM_STEP);
+  mio->pointer = xzalloc (MEM_STEP);
   mio->size = MEM_STEP;
   mio->cur = 0;
   mio->flags = flags;
@@ -93,9 +95,13 @@ ios_dev_mem_putc (void *iod, int c)
 {
   struct ios_dev_mem *mio = iod;

-  if (mio->cur >= mio->size)
+  if (mio->cur >= mio->size) {
+    assert (mio->cur - mio->size < MEM_STEP);
     mio->pointer = xrealloc (mio->pointer,
                              mio->size + MEM_STEP);
+    memset (&mio->pointer[mio->size], 0, MEM_STEP);
+    mio->size += MEM_STEP;
+  }
   mio->pointer[mio->cur++] = c;
   return c;
 }
@@ -111,21 +117,25 @@ static int
 ios_dev_mem_seek (void *iod, ios_dev_off offset, int whence)
 {
   struct ios_dev_mem *mio = iod;
+  size_t target = mio->cur;

   switch (whence)
     {
     case IOD_SEEK_SET:
-      mio->cur = offset;
+      target = offset;
       break;
     case IOD_SEEK_CUR:
-      mio->cur += offset;
+      target += offset;
       break;
     case IOD_SEEK_END:
-      mio->cur = mio->size - offset;
+      target = mio->size - offset;
       break;
     }

-  return mio->cur;
+  if (target >= mio->size + MEM_STEP)
+    return IOD_EOF;
+
+  return mio->cur = target;
 }

 static ios_dev_off
diff --git a/testsuite/poke.cmd/mem-1.pk b/testsuite/poke.cmd/mem-1.pk
new file mode 100644
index 00000000..c9c4f85d
--- /dev/null
+++ b/testsuite/poke.cmd/mem-1.pk
@@ -0,0 +1,6 @@
+/* { dg-do run } */
+
+/* { dg-command { .set obase 10 } } */
+/* { dg-command { .mem "*foo*" } } */
+/* { dg-command { byte[3] @ 5#B } } */
+/* { dg-output {[0,0,0]} } */
diff --git a/testsuite/poke.cmd/mem-2.pk b/testsuite/poke.cmd/mem-2.pk
new file mode 100644
index 00000000..7ab0e2d3
--- /dev/null
+++ b/testsuite/poke.cmd/mem-2.pk
@@ -0,0 +1,11 @@
+/* { dg-do run } */
+
+/* { dg-command { .set obase 10 } } */
+/* { dg-command { .mem "*foo*" } } */
+/* { dg-command { iosize (0) } } */
+/* { dg-output "32768UL#b" } */
+/* { dg-command { byte @ 4096#B = 1 } } */
+/* { dg-command { iosize (0) } } */
+/* { dg-output "\n65536UL#b" } */
+/* { dg-command { int @ 4096#B } } */
+/* { dg-output "\n16777216" } */
diff --git a/testsuite/poke.cmd/mem-3.pk b/testsuite/poke.cmd/mem-3.pk
new file mode 100644
index 00000000..95c11bdf
--- /dev/null
+++ b/testsuite/poke.cmd/mem-3.pk
@@ -0,0 +1,6 @@
+/* { dg-do run } */
+
+/* { dg-command { .set obase 10 } } */
+/* { dg-command { .mem "*foo*" } } */
+/* { dg-command { try byte @ 1024 * 1024#B = 1; catch if E_eof { printf 
"caught\n"; } } } */
+/* { dg-output "caught" } */
-- 
2.25.1