Fix 32-bit overflow in Divesoft Freedom time handling

Commit 31fb2e4c62ab ("Avoid possible sign extension") handled the
problem when a "unsigned char" is shifted 24 bits left, and becomes a
"signed int".  By casting the result to uint32_t, that signed case won't
happen.

However, there were two bugs in that fix.

The first is the comment.  It's not that "timestamp_t" is signed that is
the problem.  No, the problem is inherent in the C expression

    (ptr[11] << 24)

where "ptr[11]" is an unsigned char.  In C arithmetic, unsigned char is
implicitly type-expanded to "int", so while it has a value between
0..255, when you shift it left by 24, you can get a *negative* "int" as
a result.

So it's actually "ptr[11]" that should have been cast to "unsigned", but
it so happens that you can do all the shifting and adding in "int", and
then cast the end result to "uint32_t" and you'll get the same value.
But at no point did "timestamp_t" matter.

The other bug was pre-existing and just not fixed.  When the code does
the "+ 946684800" (to turn the timestamp to be seconds from the start of
2000, into seconds since the "unix epoch", ie 1970) that arithmetic is
now done in that "uint32_t" (and used to be done in "int").

Which means that the addition can overflow in 32 bits *before* it is
cast to timestamp_t (which is 64 bits).

Admittedly that 32-bit overflow happens a bit later than the sign bit
gets set, but if we're worried aboout overflows, let's just do this
right.

In other words, we have a 32-bit unsigned offset since Jan 1, 2000, and
for the full range we need to do the epoch correction in 32 bits.
Because otherwise you fail in the year 2106 (32-bit unsigned unix epoch
time limit), even though the 32-bit seconds *should* work all the way
until the year 2136.

Of course, I'll be rather surprised if people still use the Divesoft
Freedom in the year 2106.  Or rather, I won't be surprised, because I'll
be dead.

But if we think that the signed problem matters (in the year 2068), then
dammit, we can extend it another 30 years.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

1 files changed, 4 insertions, 2 deletions

diff --git a/parse-xml.c b/parse-xml.c
index 46261e9c9..1aaeb42fe 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -3269,9 +3269,11 @@ int parse_dlf_buffer(unsigned char *buffer, size_t size)
 	snprintf(serial, sizeof(serial), "%d", (ptr[7] << 8) + ptr[6]);
 	cur_dc->serial = strdup(serial);
 	// Dive start time in seconds since 2000-01-01 00:00
-	// let's avoid implicit sign extensions (as timestamp_t is signed)
+	// let's avoid implicit sign extensions
+	// ("unsigned char" is expanded to "int" in C arithmetic, so
+	// (ptr[11] << 24) can be a signed integer)
 	uint32_t offset = (ptr[11] << 24) + (ptr[10] << 16) + (ptr[9] << 8) + ptr[8];
-	cur_dc->when = offset + 946684800;
+	cur_dc->when = (timestamp_t) offset + 946684800;
 	cur_dive->when = cur_dc->when;
 
 	cur_dc->duration.seconds = ((ptr[14] & 0xFE) << 16) + (ptr[13] << 8) + ptr[12];