Skip to content

feature/adm: fix out-of-bounds read at scale 3 for frame dimensions 17 to 32 - #1599

Open
lusoris wants to merge 1 commit into
Netflix:masterfrom
VMAFx:fix/adm-dwt2-index-small-inputs
Open

lusoris wants to merge 1 commit into
Netflix:masterfrom
VMAFx:fix/adm-dwt2-index-small-inputs

Conversation

@lusoris

@lusoris lusoris commented Sep 19, 2026 •

Copy link
Copy Markdown

dwt2_src_indices_filt() fills the DWT source-index table in three ranges: output 0, the interior, and a mirrored tail that starts at n_half - 2. For an input of 3 or 4 samples n_half is 2, so the tail restarts at output 0 and replaces its { 1, 0, 1, 2 } with { -1, 0, 1, 2 }. adm_dwt2_s123_combined() and its AVX2 and AVX-512 versions then read row -1 and column -1 of their input. Scale 3 sees a 3- or 4-sample input for any frame width or height from 17 to 32. Column -1 is the int32_t in front of the tmp_ref allocation, which AddressSanitizer reports as a heap-buffer-overflow; row -1 lies inside data_buf, so it only shows as a wrong score.

Start the tail at 1 when n_half is 2. The table is unchanged for every larger input. The float implementation (dwt2_src_indices_filt_s()) mirrors index -1 to 1 and is not affected.

The regression test test_adm_dwt2_indices compares every table entry for inputs of 3 to 66 samples with the symmetric extension. On the unpatched tree it fails with n=3 output=0 tap=0: index -1, expected 1.

Reproducer, using the first 2592 bytes of the src01 test clips as three 24x24 frames:

head -c 2592 python/test/resource/yuv/src01_hrc00_576x324.yuv > ref_24x24.yuv
head -c 2592 python/test/resource/yuv/src01_hrc01_576x324.yuv > dis_24x24.yuv
build-sanitize/tools/vmaf -r ref_24x24.yuv -d dis_24x24.yuv -w 24 -h 24 -p 420 -b 8 \
  --feature adm --no_prediction --cpumask -1 -q -o /dev/null
# ==ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 4
#     #0 adm_dwt2_s123_combined ../libvmaf/src/feature/integer_adm.c:2752
#     #1 integer_compute_adm ../libvmaf/src/feature/integer_adm.c:2948
# ... is located 4 bytes before 384-byte region ... allocated by adm_buffer_alloc ../libvmaf/src/feature/integer_adm.c:3071
# Without --cpumask the same report names adm_dwt2_s123_combined_avx512 (adm_avx512.c:2705).

Over single-frame synthetic 8-bit clips with widths 17 to 34 and heights 17, 18, 24, 31, 32, 33 and 40, each under the default dispatch, --cpumask 16 and --cpumask -1, 336 of 378 runs end in that report on the unpatched tree (every run with a width of 17 to 32) and none with the patch.

Effect on integer_adm_scale3 for the clip above, with float_adm as the reference:

frame 0 frame 1 frame 2
unpatched 0.925844 0.920199 0.913376
patched 0.986957 0.973522 0.945018
float_adm adm_scale3 0.986972 0.973548 0.944989

Scales 0 to 2 are identical before and after; integer_adm2 follows scale 3.

Rebased on master 9e48141 (2026-10-02).

Validation against upstream 9e48141bd1eb8d2329e09d3744e7c24af53017ca, x86-64 Linux (AVX-512 host), GCC 16.2.1, Meson 1.12.1:

meson setup build-release libvmaf --buildtype=release -Denable_cuda=false -Denable_float=true -Denable_docs=false
ninja -C build-release
meson test -C build-release --print-errorlogs
# 25/25 passed

meson setup build-sanitize libvmaf --buildtype=debug -Db_sanitize=address,undefined -Denable_cuda=false -Denable_float=true -Denable_docs=false
ninja -C build-sanitize
ASAN_OPTIONS=detect_leaks=1:halt_on_error=1 UBSAN_OPTIONS=halt_on_error=1 meson test -C build-sanitize test_adm_dwt2_indices test_feature_extractor --print-errorlogs
# both passed

No golden value changes. The three reference pairs (src01_hrc00/src01_hrc01 at 576x324, and checkerboard_1920_1080_10_3_0_0 against _1_0 and _10_0) give identical per-frame and pooled JSON output from the unpatched and the patched release build, under the default dispatch and with --cpumask -1; pooled vmaf_v0.6.1 means are 76.667831, 35.068667 and 7.985899 in all cases.

This does not make frames below 17 pixels work. A 16x16 frame still fails after this patch: UBSan reports shift exponent 4294967295 in adm_cm() and the release build crashes. The remaining UBSan reports in the size sweep (shift exponent -1 is negative in get_best15_from32(), adm_avx2.c:1350) are present before and after and are not touched here.

…7 to 32

dwt2_src_indices_filt() builds its index table in three ranges: output 0,
the interior, and a mirrored tail that starts at n_half - 2. For an input
of 3 or 4 samples n_half is 2, so the tail restarts at output 0 and
replaces its { 1, 0, 1, 2 } with { -1, 0, 1, 2 }. adm_dwt2_s123_combined()
then reads row -1 and column -1. Scale 3 sees such an input for any frame
width or height from 17 to 32.

Start the tail at 1 when n_half is 2. Larger inputs are unaffected.

Add test_adm_dwt2_indices, which checks every index for inputs of 3 to 66
samples against the symmetric extension.
@lusoris
lusoris force-pushed the fix/adm-dwt2-index-small-inputs branch from 5241b3b to d60bc79 Compare October 2, 2026 18:38
lusoris added a commit to VMAFx/vmafx that referenced this pull request Oct 2, 2026
…each with its size and the upstream change that ends it (ADR-1479 to ADR-1486)

The reference for code inherited from Netflix/vmaf is Netflix's source;
a difference needs an ADR. The upstream parity audit of 2026-10-02 found
deliberate differences that had none of their own, or whose ADR
(ADR-1033) names neither upstream's behaviour nor the size:

- ADR-1479 ciede on 4:2:2: chroma flags (fork PR #1050); 0.153 on 48 of
  48 frames; Netflix/vmaf#1611.
- ADR-1480 speed_temporal buffers at speed_prescale above 1 (#1643);
  up to 195, upstream segfaults on two fixtures; Netflix/vmaf#1627.
- ADR-1481 a failing extractor fails the run (#871); status only, 78
  probe runs where upstream is silent and 88 where it crashes.
- ADR-1482 integer adm on frames of 17 to 32 pixels (#1473, #1507);
  scale 3 up to 0.23; Netflix/vmaf#1599, #1600.
- ADR-1483 odd-sized chroma planes round up (4f08d32); psnr_cb / cr
  up to 0.684 / 0.826 dB, ciede 0.198.
- ADR-1484 float_ms_ssim magnitude before pow() (#641, ADR-1033 item 2);
  NaN upstream on the 10 px checkerboard; Netflix/vmaf#1665.
- ADR-1485 apsnr of a plane without error (#641, item 1); 114 against
  60 dB; Netflix/vmaf#1666.
- ADR-1486 float_motion scale-1 stride (#641, item 9); up to 25.1;
  Netflix/vmaf#1667.

Each ADR gives upstream's file and line at Netflix 9e48141b, the fork's
lines, the reason found in the fork's pull request, commit or code, and
the measured size from the audit. Documentation only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant