tag 702392 confirmed patch
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu ubuntu-patch saucy
thanks

Vincent Danjean [2013-03-06  0:53 +0100]:
> dosfslabel segfault when creating a label. I tested it on real device and
> on a file (formated with mkdosfs before). Downgrading to 3.0.13-1 (current
> version in testing) fix the problem.

Re-confirmed against current upstream git head. This currently fails
to build for me:

cc   dosfslabel.o boot.o check.o common.o fat.o file.o io.o lfn.o charconv.o   
-o dosfslabel
boot.o: In function `write_volume_label':
dosfstools/src/boot.c:560: undefined reference to `cpu_to_le16'
dosfstools/src/boot.c:561: undefined reference to `cpu_to_le16'
dosfstools/src/boot.c:562: undefined reference to `cpu_to_le32'
collect2: error: ld returned 1 exit status

The first attached patch fixes this.

The actual crash happens in write_label() in boot.c:

----- 8< ------
{
    int l = strlen(label);

    while (l < 11)
        label[l++] = ' ';
----- 8< ------

src/dosfslabel.c calls write_label() with the plain argv argument
without making a copy; so write_label() must not modify this. With
above code it does and overflows argv[1].

The second attached patch fixes this by making a copy and ensuring
that this is right-filled with space.

Thanks,

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From 593faa093bdd60cd562d347fdfee26e20570ff30 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martinp...@gnome.org>
Date: Fri, 24 May 2013 06:52:03 +0200
Subject: [PATCH 1/2] Clean up remaining byteswap code.

Replace the remaining three CT_LE_* macro calls with htole*, completing commit
f3bd63.

This fixes the build failure:

   boot.o: In function `write_volume_label':
   src/boot.c:560: undefined reference to `cpu_to_le16'
---
 src/boot.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/boot.c b/src/boot.c
index b7c4e4d..86cc682 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -557,9 +557,9 @@ static void write_volume_label(DOS_FS * fs, char *label)
       de.ctime = de.time;
       de.cdate = de.date;
       de.adate = de.date;
-      de.starthi = CT_LE_W(0);
-      de.start = CT_LE_W(0);
-      de.size = CT_LE_L(0);
+      de.starthi = htole16(0);
+      de.start = htole16(0);
+      de.size = htole32(0);
     }
 
     fs_write(offset, sizeof(DIR_ENT), &de);
-- 
1.8.1.2

From e3947b51f36c939ebbc61f3962964c0dbbe340a6 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martinp...@gnome.org>
Date: Fri, 24 May 2013 06:45:57 +0200
Subject: [PATCH 2/2] Fix memory corruption with writing labels

In write_label(), do not assume that the passed label is big enough to actually
hold 11 characters. dosfslabel just passes the argv[] string, which must not
be modified; so create a copy for filling up with spaces.

See http://bugs.debian.org/702392 and https://launchpad.net/bugs/1183406
---
 src/boot.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/boot.c b/src/boot.c
index 86cc682..4b22bbe 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -567,11 +567,10 @@ static void write_volume_label(DOS_FS * fs, char *label)
 
 void write_label(DOS_FS * fs, char *label)
 {
-    int l = strlen(label);
+    /* we want to fill up the passed label with spaces to 11 chars */
+    char filled_label[] = "           ";
+    strncpy(filled_label, label, sizeof(filled_label) - 1);
 
-    while (l < 11)
-	label[l++] = ' ';
-
-    write_boot_label(fs, label);
-    write_volume_label(fs, label);
+    write_boot_label(fs, filled_label);
+    write_volume_label(fs, filled_label);
 }
-- 
1.8.1.2

Attachment: signature.asc
Description: Digital signature

Reply via email to