AJ0107 causes read from OOB memory region
AJ0107 increased the shape of m_aiGpmIntraMPMLists from [64][2][3] to [112][2][3]. IntraPrediction::setPrefilledIntraGPMMPMModeAll copies sizeof(m_aiGpmIntraMPMLists) bytes from the address provided in its mpm parameter to m_aiGpmIntraMPMLists. IntraPrediction::setPrefilledIntraGPMMPMModeAll is only called at one site in InterSearch::initGeoAngleSelection which is in turn only called at one site in EncCu::xCheckRDCostMergeGeoComb2Nx2N, where the mpm argument is the address of the local array geoIntraMPMList. AJ0107 failed to increase the size of this local array geoIntraMPMList correspondingly with the number of bytes read from it, thus data from an out-of-bounds memory region is copied into m_aiGpmIntraMPMLists.
Below is an address sanitizer report
=================================================================
==42161==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x00016b32a730 at pc 0x00011d2abd88 bp 0x00016b2a9790 sp 0x00016b2a8f40
READ of size 672 at 0x00016b32a730 thread T0
#0 0x00011d2abd84 in __asan_memcpy+0x44c (libclang_rt.asan_osx_dynamic.dylib:arm64+0x57d84)
#1 0x0001056f684c in InterSearch::initGeoAngleSelection(PredictionUnit&, IntraPrediction*, unsigned char const (&) [64][2][3]) InterSearch.cpp:8305
#2 0x0001054c5f8c in EncCu::xCheckRDCostMergeGeoComb2Nx2N(CodingStructure*&, CodingStructure*&, Partitioner&, EncTestMode const&, bool) EncCu.cpp:12220
#3 0x000105472d30 in EncCu::xCompressCU(CodingStructure*&, CodingStructure*&, Partitioner&, double, int) EncCu.cpp:1860
#4 0x000105525acc in EncCu::xCheckModeSplit(CodingStructure*&, CodingStructure*&, Partitioner&, EncTestMode const&, double*, int) EncCu.cpp:2956
#5 0x000105473278 in EncCu::xCompressCU(CodingStructure*&, CodingStructure*&, Partitioner&, double, int) EncCu.cpp:2126
#6 0x000105525acc in EncCu::xCheckModeSplit(CodingStructure*&, CodingStructure*&, Partitioner&, EncTestMode const&, double*, int) EncCu.cpp:2956
#7 0x000105473278 in EncCu::xCompressCU(CodingStructure*&, CodingStructure*&, Partitioner&, double, int) EncCu.cpp:2126
#8 0x00010546e97c in EncCu::compressCtu(CodingStructure&, UnitArea const&, unsigned int, int const*, int const*) EncCu.cpp:774
#9 0x00010567c864 in EncSlice::encodeCtus(Picture*, bool, bool, EncLib*) EncSlice.cpp:2407
#10 0x000105675ba0 in EncSlice::compressSlice(Picture*, bool, bool) EncSlice.cpp:1812
#11 0x0001055888fc in EncGOP::compressGOP(int, int, std::__1::list<Picture*, std::__1::allocator<Picture*>>&, std::__1::list<UnitBuf<short>*, std::__1::allocator<UnitBuf<short>*>>&, bool, bool, InputColourSpaceConversion, bool, bool, bool, int) EncGOP.cpp:3693
#12 0x0001055ed0e0 in EncLib::encode(InputColourSpaceConversion, std::__1::list<UnitBuf<short>*, std::__1::allocator<UnitBuf<short>*>>&, int&) EncLib.cpp:1251
#13 0x000104a6bfec in EncApp::encode() EncApp.cpp:1980
#14 0x000104b43460 in main encmain.cpp:281
#15 0x000187f1f150 (<unknown module>)
#16 0xf5b7ffffffffffc (<unknown module>)
Address 0x00016b32a730 is located in stack of thread T0 at offset 525200 in frame
#0 0x0001054bce9c in EncCu::xCheckRDCostMergeGeoComb2Nx2N(CodingStructure*&, CodingStructure*&, Partitioner&, EncTestMode const&, bool) EncCu.cpp:10877
This frame has 436 object(s):
[32, 40) 'el.addr.i.i.i24215'
[64, 72) 'el.addr.i.i.i'
[96, 176) 'ref.tmp.i24177'
[208, 288) 'ref.tmp.i24134'
[320, 400) 'ref.tmp.i24112'
// 426 objects omitted for conciseness.
[910176, 910264) 'ref.tmp16585' (line 18675)
[910304, 910392) 'ref.tmp16588' (line 18675)
[910432, 910464) 'ref.tmp16610' (line 18680)
[910496, 910520) 'ref.tmp16611' (line 18680)
[910560, 910561) 'ref.tmp16676' (line 18690)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow InterSearch.cpp:8305 in InterSearch::initGeoAngleSelection(PredictionUnit&, IntraPrediction*, unsigned char const (&) [64][2][3])
Shadow bytes around the buggy address:
0x00016b32a480: f2 f2 f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
0x00016b32a500: f8 f2 f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8
0x00016b32a580: f8 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00
0x00016b32a600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00016b32a680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00016b32a700: 00 00 00 00 00 00[f2]f2 f2 f2 f2 f2 f2 f2 00 00
0x00016b32a780: 00 00 00 00 00 00 03 f2 f2 f2 f2 f2 00 00 00 00
0x00016b32a800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x00016b32a880: 00 00 00 00 00 00 00 00 00 00 00 00 00 04 f2 f2
0x00016b32a900: f2 f2 f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8
0x00016b32a980: f8 f2 f2 f2 f2 f2 f8 f8 f8 f2 f2 f2 f2 f2 f8 f8
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==42161==ABORTING
I don't think it will be sufficient to simply increase the size of the geoIntraMPMList -- the subsequent logic in EncCu::xCheckRDCostMergeGeoComb2Nx2N makes pretty heavy use of the smaller GEO_NUM_PARTITION_MODE. I am not personally familiar enough with this code to implement a fix myself then, so CC @kevin.reuze as the commit author.