Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
Control: tags -1 -moreinfo I believe I have provided all the requested information, and is unsure how much of my proposed changes is accepted by the release managers. Can someone let me know what the status of this proposal is? -- Happy hacking Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Tue, Jul 5, 2016 at 10:37:41 +0200, Petter Reinholdtsen wrote: > [Andreas Bombe] > > If you strongly expect it to be accepted as it is, then push it. > > > > Or wait with tagging until it is accepted. Moving tags and releases > > that aren't releases after all is something I'd like to avoid. > > Right. I believe the changes are sound and suspect the release team > agree with me that fixing also minor security issues in Stable is > important. Because of this, I've pushed the changes, but not added the > tag. I suspect it would help if you the package maintainer believe > these changes should go into stable too. > I'm not sure I agree with that. To me, "minor" kind of implies "not important". Cheers, Julien
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Adam D. Barratt]
> I intentionally asked for a debdiff, not a pointer to a repository. Bug
> reports should stand alone and not be reliant on external resources
> which may change or disappear.
>
> Is the diff in <2fla8ikrwpn@diskless.uio.no> still current?
I did not provide a debdiff, because the diff is still current, if you
only want the CVEs fixed, and I did not know if that was the case. If
in addition the extra issue with invalid months reads should be fixed,
the attached patch should solve it. It is the change currently in the
debian/jessie branch in git. The only change between the two is the
upstr-11-out-of-bounds.diff file and associated updates to serial and
changelog.
So the questions I hope the release managers can answer is this: Is it
ok to update dosfstools with either of the two proposed patches? If
not, should we limit it to only those fixing CVEs?
--
Happy hacking
Petter Reinholdtsen
diff --git a/debian/changelog b/debian/changelog
index 4f1e009..44f2105 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+dosfstools (3.0.27-1+deb8u1) unstable; urgency=medium
+
+ * Non-maintainer upload to fix security issue.
+ * Added d/gbp.conf to document git branch used for Jessie updates.
+ * [CVE-2015-8872] Invalid memory read in fsck.vfat
+ * [CVE-2016-4804] Heap overflow in function read_fat()
+ * Added upstr-11-out-of-bounds.diff to avoid invalid memory read in
+fsck.fat when month is negative.
+
+ -- Petter Reinholdtsen Mon, 13 Jun 2016 08:17:24 +0200
+
dosfstools (3.0.27-1) unstable; urgency=medium
* New upstream version 3.0.27
diff --git a/debian/gbp.conf b/debian/gbp.conf
new file mode 100644
index 000..3926a07
--- /dev/null
+++ b/debian/gbp.conf
@@ -0,0 +1,3 @@
+[DEFAULT]
+debian-branch = debian/jessie
+pristine-tar = True
diff --git a/debian/patches/CVE-2015-8872.diff b/debian/patches/CVE-2015-8872.diff
new file mode 100644
index 000..8709cc4
--- /dev/null
+++ b/debian/patches/CVE-2015-8872.diff
@@ -0,0 +1,33 @@
+Description: Fix CVE-2015-8872 using patches from upstream.
+
+ The patch is based on file used to update the CVE in Wheezy. It
+ contained the fix in
+ https://github.com/dosfstools/dosfstools/commit/39ce90fe75661ed8842551cd44ea7fec278a60a1
+ Then the dosfstools maintainer noticed the patch in
+ https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
+ was missing. It is included here (off by one error, fixed by using
+ +1 instead of -1.
+
+ See also https://bugs.debian.org/827160 .
+
+Index: dosfstools-collab/src/fat.c
+===
+--- dosfstools-collab.orig/src/fat.c 2016-06-13 08:07:44.669688617 +0200
dosfstools-collab/src/fat.c 2016-06-13 08:07:44.665688587 +0200
+@@ -197,10 +197,12 @@
+ data[1] = new >> 4;
+ } else {
+ FAT_ENTRY subseqEntry;
+- get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ if (cluster != fs->clusters + 1)
++ get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ else
++ subseqEntry.value = 0;
+ data[0] = new & 0xff;
+- data[1] = (new >> 8) | (cluster == fs->clusters - 1 ? 0 :
+-(0xff & subseqEntry.value) << 4);
++ data[1] = (new >> 8) | ((0xff & subseqEntry.value) << 4);
+ }
+ size = 2;
+ break;
+
diff --git a/debian/patches/CVE-2016-4804.diff b/debian/patches/CVE-2016-4804.diff
new file mode 100644
index 000..d28174c
--- /dev/null
+++ b/debian/patches/CVE-2016-4804.diff
@@ -0,0 +1,64 @@
+https://github.com/dosfstools/dosfstools/commit/e8eff147e9da1185f9afd5b25948153a3b97cf52
+
+Index: dosfstools-collab/src/boot.c
+===
+--- dosfstools-collab.orig/src/boot.c 2016-06-13 07:59:10.337694024 +0200
dosfstools-collab/src/boot.c 2016-06-13 08:00:46.290436480 +0200
+@@ -101,8 +101,8 @@
+ (unsigned long long)fs->fat_start,
+ (unsigned long long)fs->fat_start / lss);
+ printf("%10d FATs, %d bit entries\n", b->fats, fs->fat_bits);
+-printf("%10d bytes per FAT (= %u sectors)\n", fs->fat_size,
+- fs->fat_size / lss);
++printf("%10lld bytes per FAT (= %llu sectors)\n", (long long)fs->fat_size,
++ (long long)fs->fat_size / lss);
+ if (!fs->root_cluster) {
+ printf("Root directory starts at byte %llu (sector %llu)\n",
+ (unsigned long long)fs->root_start,
+@@ -326,7 +326,7 @@
+ struct boot_sector b;
+ unsigned total_sectors;
+ unsigned short logical_sector_size, sectors;
+-unsigned fat_length;
++off_t fat_length;
+ loff_t data_size;
+
+ fs_read(0, sizeof(b), &b);
+@@ -354,8 +354,12 @@
+ /* Can't access last odd sector anyway, so round down */
+ fs_test((loff_t) ((total_sectors & ~1) - 1) * (loff_t) logical_sector_size,
+ logical_sector_size);
++
+ fat_length = le16toh(b.fat_length) ?
+ le16toh(b.fat_length) : le32toh(b.fat32_length);
++if (!fat_length)
++die("FAT size is zero.");
++
+ fs
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Thu, 2016-07-07 at 21:45 +0200, Petter Reinholdtsen wrote:
> [Adam D. Barratt]
> > The version in unstable has:
> >
> > if (month < 0) {
> > /* make sure that nothing bad happens if the month bits were zero */
> > month = 0;
> > }
> >
> > which seems like a more minimal-style change than the assert. Could we
> > have an updated debdiff including that change, please?
>
> You misunderstand. My proposed patch for jessie do not use the
> assert(). I used the assert() to make sure the bug was present, as I
> could not get it to crash with a negative month value. It was only used
> in test code.
Okay.
> The changes I propose for Jessie can be seen via
> https://anonscm.debian.org/cgit/collab-maint/dosfstools.git/log/?h=debian/jessie
> >.
I intentionally asked for a debdiff, not a pointer to a repository. Bug
reports should stand alone and not be reliant on external resources
which may change or disappear.
Is the diff in <2fla8ikrwpn@diskless.uio.no> still current?
Regards,
Adam
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Adam D. Barratt]
> The version in unstable has:
>
> if (month < 0) {
> /* make sure that nothing bad happens if the month bits were zero */
> month = 0;
> }
>
> which seems like a more minimal-style change than the assert. Could we
> have an updated debdiff including that change, please?
You misunderstand. My proposed patch for jessie do not use the
assert(). I used the assert() to make sure the bug was present, as I
could not get it to crash with a negative month value. It was only used
in test code.
The changes I propose for Jessie can be seen via
https://anonscm.debian.org/cgit/collab-maint/dosfstools.git/log/?h=debian/jessie
>.
--
Happy hacking
Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Sat, 2016-06-18 at 09:21 +0200, Petter Reinholdtsen wrote:
> [Andreas Bombe]
> diff --git a/src/check.c b/src/check.c
> index e8aaf92..086b923 100644
> --- a/src/check.c
> +++ b/src/check.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
>
> #include "common.h"
> #include "fsck.fat.h"
> @@ -236,6 +237,7 @@ static time_t date_dos2unix(unsigned short time, unsigned
> short date)
> time_t secs;
>
> month = ((date >> 5) & 15) - 1;
> +assert(month >= 0);
The version in unstable has:
if (month < 0) {
/* make sure that nothing bad happens if the month bits were zero */
month = 0;
}
which seems like a more minimal-style change than the assert. Could we
have an updated debdiff including that change, please?
Regards,
Adam
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Andreas Bombe] > If you strongly expect it to be accepted as it is, then push it. > > Or wait with tagging until it is accepted. Moving tags and releases > that aren't releases after all is something I'd like to avoid. Right. I believe the changes are sound and suspect the release team agree with me that fixing also minor security issues in Stable is important. Because of this, I've pushed the changes, but not added the tag. I suspect it would help if you the package maintainer believe these changes should go into stable too. Btw, do you know why there is no BTS report about these two CVEs? -- Happy hacking Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Sun, Jun 26, 2016 at 09:27:57AM +0200, Petter Reinholdtsen wrote: > > Andreas, while I wait for a reply from the release managers, it would be > great to know the answer to this question: > > [Petter Reinholdtsen] > > OK to push it to the collab-maint git repo before upload, or should I > > wait until it is accepted? If you strongly expect it to be accepted as it is, then push it. Or wait with tagging until it is accepted. Moving tags and releases that aren't releases after all is something I'd like to avoid.
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
Andreas, while I wait for a reply from the release managers, it would be great to know the answer to this question: [Petter Reinholdtsen] > OK to push it to the collab-maint git repo before upload, or should I > wait until it is accepted? -- Happy hacking Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Sat, Jun 18, 2016 at 09:21:58AM +0200, Petter Reinholdtsen wrote: > [Andreas Bombe] > > Also, I wonder if the fix for > > https://github.com/dosfstools/dosfstools/issues/11 (which is > > 2aad1c83c) shouldn't also be included while we're at it. It has no > > CVE, the out of bounds memory access itself isn't all that bad but it > > might create improper date values. > > > > https://github.com/dosfstools/dosfstools/commit/2aad1c83c7d010de36afbe79c9fde22c50aa2f74 > > It is fine with me, but it is up to the release managers. Is there a > Debian bug about this? I believe it is a requirement for getting a fix > into stable. > > Is this error supposed to be detected by Valgrind? I was unable to get > any warning about out of bounds memory access by valgrind when I tested. I think there was one issue that only showed up as a memory error on i386 and not amd64 and this might have been the one. Also on second thought, never mind… This date conversion is used only to display information about files, so worst case is that index -1 of a static array is read and a nonsensical date is displayed to the user. I don't think it's worth extra effort to include it. Andreas
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Andreas Bombe] > I didn't look closely when the wheezy update was uploaded, so it looks > like it missed it. > > For reference, this is the original report including a test file: > https://github.com/dosfstools/dosfstools/issues/12 > > The problem is fixed if fsck'ing that file under valgrind shows no > valgrind memory errors. Crashing without valgrind is not guaranteed. Ah, very good. I verified that the new patch get rid of the valgrind errors. > Also, I wonder if the fix for > https://github.com/dosfstools/dosfstools/issues/11 (which is > 2aad1c83c) shouldn't also be included while we're at it. It has no > CVE, the out of bounds memory access itself isn't all that bad but it > might create improper date values. > > https://github.com/dosfstools/dosfstools/commit/2aad1c83c7d010de36afbe79c9fde22c50aa2f74 It is fine with me, but it is up to the release managers. Is there a Debian bug about this? I believe it is a requirement for getting a fix into stable. Is this error supposed to be detected by Valgrind? I was unable to get any warning about out of bounds memory access by valgrind when I tested. I suspect it is because of the memory layout hiding the issue, but am not sure. Adding assert() like this did however prove the problem exist: diff --git a/src/check.c b/src/check.c index e8aaf92..086b923 100644 --- a/src/check.c +++ b/src/check.c @@ -29,6 +29,7 @@ #include #include #include +#include #include "common.h" #include "fsck.fat.h" @@ -236,6 +237,7 @@ static time_t date_dos2unix(unsigned short time, unsigned short date) time_t secs; month = ((date >> 5) & 15) - 1; +assert(month >= 0); year = date >> 9; secs = (time & 31) * 2 + 60 * ((time >> 5) & 63) + (time >> 11) * 3600 + -- Happy hacking Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Fri, Jun 17, 2016 at 06:37:03AM +0100, Adam D. Barratt wrote: > On Fri, 2016-06-17 at 05:00 +0200, Andreas Bombe wrote: > > On Mon, Jun 13, 2016 at 09:26:52AM +0200, Petter Reinholdtsen wrote: > [...] > > > https://security-tracker.debian.org/tracker/CVE-2016-4804 > > > > https://security-tracker.debian.org/tracker/CVE-2016-4804 >. > > > > > > The issues were fixed in Wheezy by the LTS team (DLA-474-1) and is also > > > fixed in unstable. I would like to get it fixed in stable too, to get > > > it out of my debsecan list. > > > > > > The attached patch is based on the patches in wheezy, and should solve > > > the problems. > > > > > > Is it OK to upload the fix for stable? > > > > Yes, please go ahead after taking into account the remark below. Thank > > you. > > Note that Andreas is not a member of the release team. Whoops, my misunderstanding of the context, sorry. On Fri, Jun 17, 2016 at 11:28:04AM +0200, Petter Reinholdtsen wrote: > [Petter Reinholdtsen] > > I will. But the comment below seem to indicate that the update in > > Wheezy was incomplete? > > Looking at the code, I am quite sure the Wheezy fix missed the change in > https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7 > >. > Who should be notified about this? I didn't look closely when the wheezy update was uploaded, so it looks like it missed it. For reference, this is the original report including a test file: https://github.com/dosfstools/dosfstools/issues/12 The problem is fixed if fsck'ing that file under valgrind shows no valgrind memory errors. Crashing without valgrind is not guaranteed. Also, I wonder if the fix for https://github.com/dosfstools/dosfstools/issues/11 (which is 2aad1c83c) shouldn't also be included while we're at it. It has no CVE, the out of bounds memory access itself isn't all that bad but it might create improper date values. https://github.com/dosfstools/dosfstools/commit/2aad1c83c7d010de36afbe79c9fde22c50aa2f74 Andreas
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Petter Reinholdtsen]
> I will. But the comment below seem to indicate that the update in
> Wheezy was incomplete?
Looking at the code, I am quite sure the Wheezy fix missed the change in
https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
>.
Who should be notified about this?
> I'll prepare a new patch and package. OK to push it to the collab-maint
> git repo before upload, or should I wait until it is accepted?
Attached is a new patch with -1 changed to +1 as instructed by the above
commit.
--
Happy hacking
Petter Reinholdtsen
diff --git a/debian/changelog b/debian/changelog
index 4f1e009..db765aa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+dosfstools (3.0.27-1+deb8u1) unstable; urgency=medium
+
+ * Non-maintainer upload to fix security issue.
+ * Added d/gbp.conf to document git branch used for Jessie updates.
+ * [CVE-2015-8872] Invalid memory read in fsck.vfat
+ * [CVE-2016-4804] Heap overflow in function read_fat()
+
+ -- Petter Reinholdtsen Mon, 13 Jun 2016 08:17:24 +0200
+
dosfstools (3.0.27-1) unstable; urgency=medium
* New upstream version 3.0.27
diff --git a/debian/gbp.conf b/debian/gbp.conf
new file mode 100644
index 000..3926a07
--- /dev/null
+++ b/debian/gbp.conf
@@ -0,0 +1,3 @@
+[DEFAULT]
+debian-branch = debian/jessie
+pristine-tar = True
diff --git a/debian/patches/CVE-2015-8872.diff b/debian/patches/CVE-2015-8872.diff
new file mode 100644
index 000..8709cc4
--- /dev/null
+++ b/debian/patches/CVE-2015-8872.diff
@@ -0,0 +1,33 @@
+Description: Fix CVE-2015-8872 using patches from upstream.
+
+ The patch is based on file used to update the CVE in Wheezy. It
+ contained the fix in
+ https://github.com/dosfstools/dosfstools/commit/39ce90fe75661ed8842551cd44ea7fec278a60a1
+ Then the dosfstools maintainer noticed the patch in
+ https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
+ was missing. It is included here (off by one error, fixed by using
+ +1 instead of -1.
+
+ See also https://bugs.debian.org/827160 .
+
+Index: dosfstools-collab/src/fat.c
+===
+--- dosfstools-collab.orig/src/fat.c 2016-06-13 08:07:44.669688617 +0200
dosfstools-collab/src/fat.c 2016-06-13 08:07:44.665688587 +0200
+@@ -197,10 +197,12 @@
+ data[1] = new >> 4;
+ } else {
+ FAT_ENTRY subseqEntry;
+- get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ if (cluster != fs->clusters + 1)
++ get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ else
++ subseqEntry.value = 0;
+ data[0] = new & 0xff;
+- data[1] = (new >> 8) | (cluster == fs->clusters - 1 ? 0 :
+-(0xff & subseqEntry.value) << 4);
++ data[1] = (new >> 8) | ((0xff & subseqEntry.value) << 4);
+ }
+ size = 2;
+ break;
+
diff --git a/debian/patches/CVE-2016-4804.diff b/debian/patches/CVE-2016-4804.diff
new file mode 100644
index 000..d28174c
--- /dev/null
+++ b/debian/patches/CVE-2016-4804.diff
@@ -0,0 +1,64 @@
+https://github.com/dosfstools/dosfstools/commit/e8eff147e9da1185f9afd5b25948153a3b97cf52
+
+Index: dosfstools-collab/src/boot.c
+===
+--- dosfstools-collab.orig/src/boot.c 2016-06-13 07:59:10.337694024 +0200
dosfstools-collab/src/boot.c 2016-06-13 08:00:46.290436480 +0200
+@@ -101,8 +101,8 @@
+ (unsigned long long)fs->fat_start,
+ (unsigned long long)fs->fat_start / lss);
+ printf("%10d FATs, %d bit entries\n", b->fats, fs->fat_bits);
+-printf("%10d bytes per FAT (= %u sectors)\n", fs->fat_size,
+- fs->fat_size / lss);
++printf("%10lld bytes per FAT (= %llu sectors)\n", (long long)fs->fat_size,
++ (long long)fs->fat_size / lss);
+ if (!fs->root_cluster) {
+ printf("Root directory starts at byte %llu (sector %llu)\n",
+ (unsigned long long)fs->root_start,
+@@ -326,7 +326,7 @@
+ struct boot_sector b;
+ unsigned total_sectors;
+ unsigned short logical_sector_size, sectors;
+-unsigned fat_length;
++off_t fat_length;
+ loff_t data_size;
+
+ fs_read(0, sizeof(b), &b);
+@@ -354,8 +354,12 @@
+ /* Can't access last odd sector anyway, so round down */
+ fs_test((loff_t) ((total_sectors & ~1) - 1) * (loff_t) logical_sector_size,
+ logical_sector_size);
++
+ fat_length = le16toh(b.fat_length) ?
+ le16toh(b.fat_length) : le32toh(b.fat32_length);
++if (!fat_length)
++die("FAT size is zero.");
++
+ fs->fat_start = (loff_t) le16toh(b.reserved) * logical_sector_size;
+ fs->root_start = ((loff_t) le16toh(b.reserved) + b.fats * fat_length) *
+ logical_sector_size;
+@@ -363,7 +367,11 @@
+ fs->data_start = fs->root_start + ROUND_TO_MULTIPLE(fs->root_entries <<
+ MSDOS_DIR_BITS,
+ logical_sector_size);
++
+ data_size = (loff_t) total_sectors *logical_sector_size - fs->data_start;
++if (data_size < fs->cluster_size)
++
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
[Andreas Bombe] > Yes, please go ahead after taking into account the remark below. Thank > you. I will. But the comment below seem to indicate that the update in Wheezy was incomplete? > This is commit 39ce90fe7 [*] which fixed a possible read access one > byte beyond the end of an array, pretty harmless since the value > wouldn't be used when the read shouldn't have happened. The following > commit 079081248 is the meatier of the fixes and the one actually > fixing the CVE (and the one referenced in the URL above). It needs to > be integrated here. I'll prepare a new patch and package. OK to push it to the collab-maint git repo before upload, or should I wait until it is accepted? -- Happy hacking Petter Reinholdtsen
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Fri, 2016-06-17 at 05:00 +0200, Andreas Bombe wrote:
> On Mon, Jun 13, 2016 at 09:26:52AM +0200, Petter Reinholdtsen wrote:
[...]
> > https://security-tracker.debian.org/tracker/CVE-2016-4804 >
> > https://security-tracker.debian.org/tracker/CVE-2016-4804 >.
> >
> > The issues were fixed in Wheezy by the LTS team (DLA-474-1) and is also
> > fixed in unstable. I would like to get it fixed in stable too, to get
> > it out of my debsecan list.
> >
> > The attached patch is based on the patches in wheezy, and should solve
> > the problems.
> >
> > Is it OK to upload the fix for stable?
>
> Yes, please go ahead after taking into account the remark below. Thank
> you.
Note that Andreas is not a member of the release team.
> > I plan to push the changes to a debian/jessie branch on collab-maint
> > once I know the changes are acceptable for a stable update.
>
>
>
> > --- /dev/null
> > +++ b/debian/patches/CVE-2015-8872.diff
> > @@ -0,0 +1,22 @@
> > +https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
> > +
> > +Index: dosfstools-collab/src/fat.c
> > +===
> > +--- dosfstools-collab.orig/src/fat.c 2016-06-13 08:07:44.669688617
> > +0200
> > dosfstools-collab/src/fat.c2016-06-13 08:07:44.665688587 +0200
> > +@@ -197,10 +197,12 @@
> > + data[1] = new >> 4;
> > + } else {
> > + FAT_ENTRY subseqEntry;
> > +- get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
> > ++ if (cluster != fs->clusters - 1)
> > ++ get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
> > ++ else
> > ++ subseqEntry.value = 0;
> > + data[0] = new & 0xff;
> > +- data[1] = (new >> 8) | (cluster == fs->clusters - 1 ? 0 :
> > +- (0xff & subseqEntry.value) << 4);
> > ++ data[1] = (new >> 8) | ((0xff & subseqEntry.value) << 4);
> > + }
> > + size = 2;
> > + break;
>
> This is commit 39ce90fe7 [*] which fixed a possible read access one byte
> beyond the end of an array, pretty harmless since the value wouldn't be
> used when the read shouldn't have happened. The following commit
> 079081248 is the meatier of the fixes and the one actually fixing the
> CVE (and the one referenced in the URL above). It needs to be integrated
> here.
>
> [*]
> https://github.com/dosfstools/dosfstools/commit/39ce90fe75661ed8842551cd44ea7fec278a60a1
If this is accurate, please can we have a new debdiff.
Regards,
Adam
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
On Mon, Jun 13, 2016 at 09:26:52AM +0200, Petter Reinholdtsen wrote:
>
> Package: release.debian.org
> Severity: normal
> Tags: jessie
> User: release.debian@packages.debian.org
> Usertags: pu
> X-Debbugs-CC: Andreas Bombe
>
> On my Debian Jessie machine, I would like to fix the two security issues
> in dosfstools that show up in the debsecan report:
> https://security-tracker.debian.org/tracker/CVE-2016-4804 >
> https://security-tracker.debian.org/tracker/CVE-2016-4804 >.
>
> The issues were fixed in Wheezy by the LTS team (DLA-474-1) and is also
> fixed in unstable. I would like to get it fixed in stable too, to get
> it out of my debsecan list.
>
> The attached patch is based on the patches in wheezy, and should solve
> the problems.
>
> Is it OK to upload the fix for stable?
Yes, please go ahead after taking into account the remark below. Thank
you.
> I plan to push the changes to a debian/jessie branch on collab-maint
> once I know the changes are acceptable for a stable update.
> --- /dev/null
> +++ b/debian/patches/CVE-2015-8872.diff
> @@ -0,0 +1,22 @@
> +https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
> +
> +Index: dosfstools-collab/src/fat.c
> +===
> +--- dosfstools-collab.orig/src/fat.c 2016-06-13 08:07:44.669688617 +0200
> dosfstools-collab/src/fat.c 2016-06-13 08:07:44.665688587 +0200
> +@@ -197,10 +197,12 @@
> + data[1] = new >> 4;
> + } else {
> + FAT_ENTRY subseqEntry;
> +-get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
> ++if (cluster != fs->clusters - 1)
> ++get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
> ++else
> ++subseqEntry.value = 0;
> + data[0] = new & 0xff;
> +-data[1] = (new >> 8) | (cluster == fs->clusters - 1 ? 0 :
> +-(0xff & subseqEntry.value) << 4);
> ++data[1] = (new >> 8) | ((0xff & subseqEntry.value) << 4);
> + }
> + size = 2;
> + break;
This is commit 39ce90fe7 [*] which fixed a possible read access one byte
beyond the end of an array, pretty harmless since the value wouldn't be
used when the read shouldn't have happened. The following commit
079081248 is the meatier of the fixes and the one actually fixing the
CVE (and the one referenced in the URL above). It needs to be integrated
here.
[*]
https://github.com/dosfstools/dosfstools/commit/39ce90fe75661ed8842551cd44ea7fec278a60a1
Bug#827160: jessie-pu: package dosfstools/3.0.27-1+deb8u1
Package: release.debian.org
Severity: normal
Tags: jessie
User: release.debian@packages.debian.org
Usertags: pu
X-Debbugs-CC: Andreas Bombe
On my Debian Jessie machine, I would like to fix the two security issues
in dosfstools that show up in the debsecan report:
https://security-tracker.debian.org/tracker/CVE-2016-4804 >
https://security-tracker.debian.org/tracker/CVE-2016-4804 >.
The issues were fixed in Wheezy by the LTS team (DLA-474-1) and is also
fixed in unstable. I would like to get it fixed in stable too, to get
it out of my debsecan list.
The attached patch is based on the patches in wheezy, and should solve
the problems.
Is it OK to upload the fix for stable?
I plan to push the changes to a debian/jessie branch on collab-maint
once I know the changes are acceptable for a stable update.
-- System Information:
Debian Release: 8.4
APT prefers stable
APT policy: (500, 'stable')
Architecture: i386 (x86_64)
Kernel: Linux 3.16.0-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=no_NO (charmap=locale: Cannot set
LC_MESSAGES to default locale: No such file or directory
locale: Cannot set LC_ALL to default locale: No such file or directory
ISO-8859-1)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
diff --git a/debian/changelog b/debian/changelog
index 4f1e009..db765aa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+dosfstools (3.0.27-1+deb8u1) unstable; urgency=medium
+
+ * Non-maintainer upload to fix security issue.
+ * Added d/gbp.conf to document git branch used for Jessie updates.
+ * [CVE-2015-8872] Invalid memory read in fsck.vfat
+ * [CVE-2016-4804] Heap overflow in function read_fat()
+
+ -- Petter Reinholdtsen Mon, 13 Jun 2016 08:17:24 +0200
+
dosfstools (3.0.27-1) unstable; urgency=medium
* New upstream version 3.0.27
diff --git a/debian/gbp.conf b/debian/gbp.conf
new file mode 100644
index 000..3926a07
--- /dev/null
+++ b/debian/gbp.conf
@@ -0,0 +1,3 @@
+[DEFAULT]
+debian-branch = debian/jessie
+pristine-tar = True
diff --git a/debian/patches/CVE-2015-8872.diff b/debian/patches/CVE-2015-8872.diff
new file mode 100644
index 000..07fb6c8
--- /dev/null
+++ b/debian/patches/CVE-2015-8872.diff
@@ -0,0 +1,22 @@
+https://github.com/dosfstools/dosfstools/commit/07908124838afcc99c577d1d3e84cef2dbd39cb7
+
+Index: dosfstools-collab/src/fat.c
+===
+--- dosfstools-collab.orig/src/fat.c 2016-06-13 08:07:44.669688617 +0200
dosfstools-collab/src/fat.c 2016-06-13 08:07:44.665688587 +0200
+@@ -197,10 +197,12 @@
+ data[1] = new >> 4;
+ } else {
+ FAT_ENTRY subseqEntry;
+- get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ if (cluster != fs->clusters - 1)
++ get_fat(&subseqEntry, fs->fat, cluster + 1, fs);
++ else
++ subseqEntry.value = 0;
+ data[0] = new & 0xff;
+- data[1] = (new >> 8) | (cluster == fs->clusters - 1 ? 0 :
+-(0xff & subseqEntry.value) << 4);
++ data[1] = (new >> 8) | ((0xff & subseqEntry.value) << 4);
+ }
+ size = 2;
+ break;
diff --git a/debian/patches/CVE-2016-4804.diff b/debian/patches/CVE-2016-4804.diff
new file mode 100644
index 000..d28174c
--- /dev/null
+++ b/debian/patches/CVE-2016-4804.diff
@@ -0,0 +1,64 @@
+https://github.com/dosfstools/dosfstools/commit/e8eff147e9da1185f9afd5b25948153a3b97cf52
+
+Index: dosfstools-collab/src/boot.c
+===
+--- dosfstools-collab.orig/src/boot.c 2016-06-13 07:59:10.337694024 +0200
dosfstools-collab/src/boot.c 2016-06-13 08:00:46.290436480 +0200
+@@ -101,8 +101,8 @@
+ (unsigned long long)fs->fat_start,
+ (unsigned long long)fs->fat_start / lss);
+ printf("%10d FATs, %d bit entries\n", b->fats, fs->fat_bits);
+-printf("%10d bytes per FAT (= %u sectors)\n", fs->fat_size,
+- fs->fat_size / lss);
++printf("%10lld bytes per FAT (= %llu sectors)\n", (long long)fs->fat_size,
++ (long long)fs->fat_size / lss);
+ if (!fs->root_cluster) {
+ printf("Root directory starts at byte %llu (sector %llu)\n",
+ (unsigned long long)fs->root_start,
+@@ -326,7 +326,7 @@
+ struct boot_sector b;
+ unsigned total_sectors;
+ unsigned short logical_sector_size, sectors;
+-unsigned fat_length;
++off_t fat_length;
+ loff_t data_size;
+
+ fs_read(0, sizeof(b), &b);
+@@ -354,8 +354,12 @@
+ /* Can't access last odd sector anyway, so round down */
+ fs_test((loff_t) ((total_sectors & ~1) - 1) * (loff_t) logical_sector_size,
+ logical_sector_size);
++
+ fat_length = le16toh(b.fat_length) ?
+ le16toh(b.fat_length) : le32toh(b.fat32_length);
++if (!fat_length)
++die("FAT size is zero.");
++
+ fs->fat_start = (loff_t) le16toh(b.reserved) * logical_sector_size;
+ fs->root_start = ((loff_t) le16toh(b.reserved) + b.fats * fat_length) *
+ logical_sector_size;
+@@ -363,7 +367,11 @@
+ fs-
