1393284 - nestegg: value is outside the range of representable values of type 'unsigned long' in [@nestegg_duration]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.webm

src/nestegg.c:2178:7: runtime error: value 1.84467e+19 is outside the range of representable values of type 'unsigned long'
    #0 0x5141c8 in nestegg_duration /srv/jenkins/jobs/nestegg-afl/workspace/default/src/nestegg.c:2178:7
    #1 0x50b97e in main /srv/jenkins/jobs/nestegg-afl/workspace/default/test/test.c:112:7
    #2 0x7fee7015e82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #3 0x419a38 in _start (/home/ubuntu/build/build/test+0x419a38)

Comment 1

[Matthew Gregan [:kinetik]]

That's:

2176   if (unscaled_duration != unscaled_duration ||
2177       unscaled_duration < 0 || unscaled_duration > (double) UINT64_MAX ||
2178       (uint64_t) unscaled_duration > UINT64_MAX / tc_scale)
2179     return -1;

...which hits __ubsan_handle_float_cast_overflow.

I expected this to be protected by |unscaled_duration > (double) UINT64_MAX|, but it turns out casting UINT64_MAX to a double rounds the value up (at least in some rounding modes, including the default), meaning it can't be round-tripped back to uint64_t safely.

Comment 2

[Matthew Gregan [:kinetik]]

Attachment: fix

Comment 3

[Ralph Giles (:rillian)]

Comment on attachment 8900549 [details] [review]
fix

Review of attachment 8900549 [details] [review]:
-----------------------------------------------------------------

::: src/nestegg.c
@@ +2174,4 @@
>      return -1;
>  
>    if (unscaled_duration != unscaled_duration ||
> +      unscaled_duration < 0 || unscaled_duration >= (double) UINT64_MAX ||

Could we have the opposite problem, where the conversion rounds down?

Comment 4

[Matthew Gregan [:kinetik]]

(In reply to Ralph Giles (:rillian) | needinfo me from comment #3)
> Could we have the opposite problem, where the conversion rounds down?

I think it'd be unusual to be running in that environment (the default is specified by IEEE 754 AFAIK), but in that case it'll be safe to convert the value to uint64_t via a cast, the only downside is that the converted duration will be limited to a slightly smaller value.

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: bug1393284_v0.patch

Import update from upstream.  Carrying review forward.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b44941defeb
Update libnestegg to 369d153.  r=rillian

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6b44941defeb