Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9289 closed defect (invalid)

ffmpeg decode aac crashed in get_bits function

Reported by: hyhmaffia Owned by:
Priority: normal Component: avcodec
Version: git-master Keywords: crash
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by hyhmaffia)

ffmpeg decode aac crashed in get_bits function on arm64 cpu
Crash Screenshot please see the attachment

the get_bits has a code : ldr w11, [x10, x11]

(lldb) register read x10

x10 = 0x000000013bbffea0

(lldb) register read x11

x11 = 0x000000000000015d

(lldb) p/x 0x000000013bbffea0 + 0x000000000000015d
(long) $12 = 0x000000013bbffffd
(lldb) p *(GetBitContext*)0x000000017048a6c0
(GetBitContext) $13 = {

buffer = 0x000000013bbffea0 "
buffer_end = 0x000000013bbffffe ""
index = 2792
size_in_bits = 2800
size_in_bits_plus8 = 2808

}
(lldb) p 0x000000013bbffffe - 0x000000013bbffffd
(long) $14 = 1
(lldb) p 0x000000013bc00000 - 0x000000013bbffffd
(long) $15 = 3

when index is 2792 and size_in_bits is 2800, we call then function get_bits(s, 3)
the get_bits will read 32 bits, but we only left 8 bits to read, so it crashed
it only crashed when the last byte in other memory page(0x000000013bc00000)

the aac packet with adts header is: (350 byte)

FFF15C802BDFFC216B44B5BA96CB40B090AC132839861B1029A6911717BBD4BC0318F0968D118EA36C32B80CEE092E16D98E230F90586D6F56CC312BD44DFCE56C2F4D08E0730B822240612F55E99BBCA15E79F9F972837C67555A4892CC4B0C70C414A838F91BE4130B2C25EFE39C126E038DB19D5A0DD0945D3EFB63F6CB19785F9CDB6515DB9E77977CECB9AE5E546D38402AAA259615B94F41255744F07666653ECA2C5954B2CBAAEBA108504B13B0C094185C55ADB763CA8550BE1175A520B949C263CBCB977E26F3946DF307DCBED83CD858AA162C1754F75B85D4FFC290EF5CD6FA4A6B56C3153BC0C456093CFB6354F09A910313F5797DE199ADEAB62272FB32A952A5E3E9A9BD18085F7B2D79247350A09F73EBC079C775A7CE7FFDA0752E19CE0006755BB1A20CD6AA6866C2440A7DCF45A0C775C8CAAA8445044D1FF70000000000006F05D12A32A26608274745FA2941D1D17E8A500000E0

Attachments (2)

C9085433-E823-44DC-9ADB-B82AFC4BC79C.png (1.3 MB ) - added by hyhmaffia 3 years ago.
Crash Screenshot
1.dat (350 bytes ) - added by hyhmaffia 3 years ago.
crash data

Download all attachments as: .zip

Change History (10)

by hyhmaffia, 3 years ago

Crash Screenshot

comment:1 by hyhmaffia, 3 years ago

Description: modified (diff)

comment:2 by Carl Eugen Hoyos, 3 years ago

Keywords: crash added; get_bits crashed removed
Priority: criticalnormal

Please attach an input file that triggers the crash.

by hyhmaffia, 3 years ago

Attachment: 1.dat added

crash data

comment:3 by hyhmaffia, 3 years ago

static inline unsigned int get_bits(GetBitContext *s, int n)
{

register unsigned int tmp;

#if CACHED_BITSTREAM_READER

av_assert2(n>0 && n<=32);
if (n > s->bits_left) {

#ifdef BITSTREAM_READER_LE

refill_32(s, 1);

#else

refill_32(s, 0);

#endif

if (s->bits_left < 32)

s->bits_left = n;

}

#ifdef BITSTREAM_READER_LE

tmp = get_val(s, n, 1);

#else

tmp = get_val(s, n, 0);

#endif
#else

OPEN_READER(re, s);
av_assert2(n>0 && n<=25);
UPDATE_CACHE(re, s);
tmp = SHOW_UBITS(re, s, n);
LAST_SKIP_BITS(re, s, n);
CLOSE_READER(re, s);

#endif

av_assert2(tmp < UINT64_C(1) << n);
return tmp;

}

the problem is, when re_index is 2792 and (gb)->buffer is 0x000000013bbffea0
the source code: tmp = NEG_USR32(re_cache, num);
will read 32bit, from 0x000000013bbffffd to 0x000000013bc00001
but valid memory range is from 0x000000013bbffffd to 0x000000013bbffffe

static inline unsigned int get_bits(GetBitContext *s, int n)
{

unsigned int re_index = (gb)->index;
unsigned int av_unused re_cache;
av_assert2(n>0 && n<=25);
AV_RB32((gb)->buffer + (re_index >> 3)) << (re_index & 7)
tmp = NEG_USR32(re_cache, num);
LAST_SKIP_BITS(re, s, n);
CLOSE_READER(re, s);

}

comment:4 by Carl Eugen Hoyos, 3 years ago

How can I reproduce the issue?

$ ffmpeg -f aac -i 1.dat -f null -
ffmpeg version N-102724-ga501d55905 Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 7 (SUSE Linux)
  configuration: --enable-gpl --toolchain=gcc-asan
  libavutil      57.  0.100 / 57.  0.100
  libavcodec     59.  1.101 / 59.  1.101
  libavformat    59.  3.100 / 59.  3.100
  libavdevice    59.  0.100 / 59.  0.100
  libavfilter     8.  0.102 /  8.  0.102
  libswscale      6.  0.100 /  6.  0.100
  libswresample   4.  0.100 /  4.  0.100
  libpostproc    56.  0.100 / 56.  0.100
[aac @ 0xffff76903c80] Estimating duration from bitrate, this may be inaccurate
Input #0, aac, from '1.dat':
  Duration: 00:00:00.05, bitrate: 60 kb/s
  Stream #0:0: Audio: aac (HE-AAC), 44100 Hz, stereo, fltp, 60 kb/s
Stream mapping:
  Stream #0:0 -> #0:0 (aac (native) -> pcm_s16le (native))
Press [q] to stop, [?] for help
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf59.3.100
  Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s
    Metadata:
      encoder         : Lavc59.1.101 pcm_s16le
size=N/A time=00:00:00.04 bitrate=N/A speed=13.3x        
video:0kB audio:8kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown

in reply to:  4 comment:5 by hyhmaffia, 3 years ago

reproduce the issue is very hard, please analyze the code and information I offered. get_bits always read 32 bit but not check the count of bit can read

comment:6 by mkver, 3 years ago

  1. The get_bits() functions are allowed to overread a bit: The buffer has to be padded. That's why we demand input packets to have at least AV_INPUT_BUFFER_PADDING_SIZE bytes of padding.
  2. The information you offered is very insufficient: You did not even provide a backtrace which is immensely helpful to check where the GetBitContext that is used in this call uses a buffer that ought to be padded. You also did not really mention your method of finding this. If you don't provide this information I just have to presume that you sent an unpadded packet to the decoder in which case the overread were neither surprising nor a bug; instead it would be a result of an API violation.

in reply to:  6 comment:7 by hyhmaffia, 3 years ago

Resolution: wontfix
Status: newclosed

we did not add padding, our mistake! sorry

comment:8 by Carl Eugen Hoyos, 3 years ago

Resolution: wontfixinvalid
Note: See TracTickets for help on using tickets.