[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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 <string.h>
 #include <limits.h>
 #include <time.h>
+#include <assert.h>
 
 #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


Reply to: