diff mbox

[v2,06/12] block/dmg: process XML plists

Message ID 1420566495-13284-7-git-send-email-peter@lekensteyn.nl
State New
Headers show

Commit Message

Peter Wu Jan. 6, 2015, 5:48 p.m. UTC
The format is simple enough to avoid using a full-blown XML parser. It
assumes that all BLKX items begin with the "mish" magic word, therefore
it is not a problem if other values get matched which are not a BLKX
block.

The offsets are based on the description at
http://newosxbook.com/DMG.html

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
 v2: added offset check, allow resource fork to be located at beginning
     of file (removed `rsrc_fork_offset != 0` condition)

It got a `Reviewed-by: John Snow <jsnow@redhat.com>` for v1. I have not
copied the R-b as I wanted to be sure that John agrees with the removal
of the offset != 0 condition.
---
 block/dmg.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

Comments

John Snow Jan. 7, 2015, 6:06 p.m. UTC | #1
On 01/06/2015 12:48 PM, Peter Wu wrote:
> The format is simple enough to avoid using a full-blown XML parser. It
> assumes that all BLKX items begin with the "mish" magic word, therefore
> it is not a problem if other values get matched which are not a BLKX
> block.
>
> The offsets are based on the description at
> http://newosxbook.com/DMG.html
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>   v2: added offset check, allow resource fork to be located at beginning
>       of file (removed `rsrc_fork_offset != 0` condition)
>
> It got a `Reviewed-by: John Snow <jsnow@redhat.com>` for v1. I have not
> copied the R-b as I wanted to be sure that John agrees with the removal
> of the offset != 0 condition.

It makes sense to me, but as I don't really have an authoritative 
standard to follow, this really relies on "This didn't break anything 
obvious."

I think you have more images than I do, and as long as you tested them 
again under v2:

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>   block/dmg.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 74 insertions(+)
>
> diff --git a/block/dmg.c b/block/dmg.c
> index 5f6976b..1a0fa0e 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -26,6 +26,7 @@
>   #include "qemu/bswap.h"
>   #include "qemu/module.h"
>   #include <zlib.h>
> +#include <glib.h>
>
>   enum {
>       /* Limit chunk sizes to prevent unreasonable amounts of memory being used
> @@ -345,12 +346,66 @@ fail:
>       return ret;
>   }
>
> +static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
> +                              uint64_t info_begin, uint64_t info_length)
> +{
> +    BDRVDMGState *s = bs->opaque;
> +    int ret;
> +    uint8_t *buffer = NULL;
> +    char *data_begin, *data_end;
> +
> +    /* Have at least some length to avoid NULL for g_malloc. Attempt to set a
> +     * safe upper cap on the data length. A test sample had a XML length of
> +     * about 1 MiB. */
> +    if (info_length == 0 || info_length > 16 * 1024 * 1024) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    buffer = g_malloc(info_length + 1);
> +    buffer[info_length] = '\0';
> +    ret = bdrv_pread(bs->file, info_begin, buffer, info_length);
> +    if (ret != info_length) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
> +    /* look for <data>...</data>. The data is 284 (0x11c) bytes after base64
> +     * decode. The actual data element has 431 (0x1af) bytes which includes tabs
> +     * and line feeds. */
> +    data_end = (char *)buffer;
> +    while ((data_begin = strstr(data_end, "<data>")) != NULL) {
> +        gsize out_len = 0;
> +
> +        data_begin += 6;
> +        data_end = strstr(data_begin, "</data>");
> +        /* malformed XML? */
> +        if (data_end == NULL) {
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +        *data_end++ = '\0';
> +        g_base64_decode_inplace(data_begin, &out_len);
> +        ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
> +                                  (uint32_t)out_len);
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +    }
> +    ret = 0;
> +
> +fail:
> +    g_free(buffer);
> +    return ret;
> +}
> +
>   static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
>                       Error **errp)
>   {
>       BDRVDMGState *s = bs->opaque;
>       DmgHeaderState ds;
>       uint64_t rsrc_fork_offset, rsrc_fork_length;
> +    uint64_t plist_xml_offset, plist_xml_length;
>       int64_t offset;
>       int ret;
>
> @@ -384,12 +439,31 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
>           ret = -EINVAL;
>           goto fail;
>       }
> +    /* offset of property list (XMLOffset) */
> +    ret = read_uint64(bs, offset + 0xd8, &plist_xml_offset);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    ret = read_uint64(bs, offset + 0xe0, &plist_xml_length);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    if (plist_xml_offset >= offset ||
> +        plist_xml_length > offset - plist_xml_offset) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }
>       if (rsrc_fork_length != 0) {
>           ret = dmg_read_resource_fork(bs, &ds,
>                                        rsrc_fork_offset, rsrc_fork_length);
>           if (ret < 0) {
>               goto fail;
>           }
> +    } else if (plist_xml_length != 0) {
> +        ret = dmg_read_plist_xml(bs, &ds, plist_xml_offset, plist_xml_length);
> +        if (ret < 0) {
> +            goto fail;
> +        }
>       } else {
>           ret = -EINVAL;
>           goto fail;
>
diff mbox

Patch

diff --git a/block/dmg.c b/block/dmg.c
index 5f6976b..1a0fa0e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -26,6 +26,7 @@ 
 #include "qemu/bswap.h"
 #include "qemu/module.h"
 #include <zlib.h>
+#include <glib.h>
 
 enum {
     /* Limit chunk sizes to prevent unreasonable amounts of memory being used
@@ -345,12 +346,66 @@  fail:
     return ret;
 }
 
+static int dmg_read_plist_xml(BlockDriverState *bs, DmgHeaderState *ds,
+                              uint64_t info_begin, uint64_t info_length)
+{
+    BDRVDMGState *s = bs->opaque;
+    int ret;
+    uint8_t *buffer = NULL;
+    char *data_begin, *data_end;
+
+    /* Have at least some length to avoid NULL for g_malloc. Attempt to set a
+     * safe upper cap on the data length. A test sample had a XML length of
+     * about 1 MiB. */
+    if (info_length == 0 || info_length > 16 * 1024 * 1024) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    buffer = g_malloc(info_length + 1);
+    buffer[info_length] = '\0';
+    ret = bdrv_pread(bs->file, info_begin, buffer, info_length);
+    if (ret != info_length) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* look for <data>...</data>. The data is 284 (0x11c) bytes after base64
+     * decode. The actual data element has 431 (0x1af) bytes which includes tabs
+     * and line feeds. */
+    data_end = (char *)buffer;
+    while ((data_begin = strstr(data_end, "<data>")) != NULL) {
+        gsize out_len = 0;
+
+        data_begin += 6;
+        data_end = strstr(data_begin, "</data>");
+        /* malformed XML? */
+        if (data_end == NULL) {
+            ret = -EINVAL;
+            goto fail;
+        }
+        *data_end++ = '\0';
+        g_base64_decode_inplace(data_begin, &out_len);
+        ret = dmg_read_mish_block(s, ds, (uint8_t *)data_begin,
+                                  (uint32_t)out_len);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+    ret = 0;
+
+fail:
+    g_free(buffer);
+    return ret;
+}
+
 static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
     BDRVDMGState *s = bs->opaque;
     DmgHeaderState ds;
     uint64_t rsrc_fork_offset, rsrc_fork_length;
+    uint64_t plist_xml_offset, plist_xml_length;
     int64_t offset;
     int ret;
 
@@ -384,12 +439,31 @@  static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto fail;
     }
+    /* offset of property list (XMLOffset) */
+    ret = read_uint64(bs, offset + 0xd8, &plist_xml_offset);
+    if (ret < 0) {
+        goto fail;
+    }
+    ret = read_uint64(bs, offset + 0xe0, &plist_xml_length);
+    if (ret < 0) {
+        goto fail;
+    }
+    if (plist_xml_offset >= offset ||
+        plist_xml_length > offset - plist_xml_offset) {
+        ret = -EINVAL;
+        goto fail;
+    }
     if (rsrc_fork_length != 0) {
         ret = dmg_read_resource_fork(bs, &ds,
                                      rsrc_fork_offset, rsrc_fork_length);
         if (ret < 0) {
             goto fail;
         }
+    } else if (plist_xml_length != 0) {
+        ret = dmg_read_plist_xml(bs, &ds, plist_xml_offset, plist_xml_length);
+        if (ret < 0) {
+            goto fail;
+        }
     } else {
         ret = -EINVAL;
         goto fail;