[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