PR #25612 opened by Kilian (lusoris) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25612 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25612.patch
Three error-path fixes in the `find_rect` filter, one commit each, found by the automated review of PR 25473 and independent of it. **Unchecked allocations.** `av_frame_clone()` and `downscale()` can return NULL, and `init()` and `filter_frame()` dereference the result. Both now fail with ENOMEM. **Coarse windows outside the image.** `filter_frame()` bounds the window against the full size frame only. With a 1920x1080 frame, a 101x101 object and 3 mipmaps, the coarsest level tests x=455 with a 26 pixel wide needle in a 480 pixel wide level, so `compare()` reads past the line. The window is now bounded at every level. **Empty coarse window.** With `xmin > xmax`, the recursive search finds nothing and leaves `sub_x` and `sub_y` uninitialized, and the caller builds its refinement window from them. It now refines only when the coarse search found a position. | Check on 17a4a474f4, x86-64 | Master | This PR | | --- | --- | --- | | Failing the Nth allocation, 699 positions | 44 crashes | 0 crashes | | Temporary abort in `compare()` on a window leaving the level, 1920x1080, 101x101 | abort at x=455 | no abort | | Same abort, 1920x1080, 100x101 | abort at y=245 | no abort | | Same abort, 320x240, 321x16 | abort at x=0 | no abort | | Same abort, 320x240, 16x241 | abort at y=0 | no abort | | MemorySanitizer with `xmin=50:xmax=10`, Clang 23 | use of uninitialized value | clean | The abort is a local test patch, not part of this PR. Builds are GCC 16 except where noted. On six inputs that stay inside the image (two sizes, 1, 3 and 5 mipmaps), the reported positions and scores are byte-identical to master. FATE has no `find_rect` test; `fate-source` passes. PR 21456 also touches this out-of-bounds read, with a check inside `compare()`. This PR bounds the search windows instead, so the two overlap and only one approach is needed. This PR also has a small textual conflict with PR 25473; whichever lands second needs a rebase, and I will do it if this one is second. >From a0d7d702abaee1df879c98147ba627257deca4b0 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 13:03:22 +0100 Subject: [PATCH 1/3] avfilter/vf_find_rect: check frame allocations av_frame_clone() and downscale() can return NULL. filter_frame() then dereferences it in the next downscale() call or in compare(), and init() does the same for the first needle level. Fail with ENOMEM instead. filter_frame() frees the levels built so far and the input frame, init() leaves the cleanup to uninit(). Signed-off-by: lusoris <[email protected]> --- libavfilter/vf_find_rect.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c index 88dfecbe222..3e21e5b4350 100644 --- a/libavfilter/vf_find_rect.c +++ b/libavfilter/vf_find_rect.c @@ -195,8 +195,18 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) foc->haystack_frame[0] = av_frame_clone(in); + if (!foc->haystack_frame[0]) { + av_frame_free(&in); + return AVERROR(ENOMEM); + } for (i=1; i<foc->mipmaps; i++) { foc->haystack_frame[i] = downscale(foc->haystack_frame[i-1]); + if (!foc->haystack_frame[i]) { + for (i = 0; i < foc->mipmaps; i++) + av_frame_free(&foc->haystack_frame[i]); + av_frame_free(&in); + return AVERROR(ENOMEM); + } } best_score = search(foc, 0, 0, @@ -272,6 +282,8 @@ static av_cold int init(AVFilterContext *ctx) } foc->needle_frame[0] = av_frame_clone(foc->obj_frame); + if (!foc->needle_frame[0]) + return AVERROR(ENOMEM); for (i = 1; i < foc->mipmaps; i++) { foc->needle_frame[i] = downscale(foc->needle_frame[i-1]); if (!foc->needle_frame[i]) -- 2.54.0 >From 91a99b3f235c4051980b278d29a9b4c4654eb4a9 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 13:03:34 +0100 Subject: [PATCH 2/3] avfilter/vf_find_rect: keep coarse search windows inside the image filter_frame() bounds the window against the full size frame only. search() halves that limit with rounding up, and downscale() rounds both the haystack and the needle size up, so a coarse level can test a position where the needle leaves the haystack. With a 1920x1080 frame, a 101x101 object and 3 mipmaps the coarsest level tests x=455 with a 26 pixel wide needle in a 480 pixel wide level. compare() then reads past the end of the line, or past the luma plane for rows. Bound the window at every level. A non-empty window stays non-empty: the coarse limit is at least half of the finer one, rounded down. Signed-off-by: lusoris <[email protected]> --- libavfilter/vf_find_rect.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c index 3e21e5b4350..9b537115494 100644 --- a/libavfilter/vf_find_rect.c +++ b/libavfilter/vf_find_rect.c @@ -153,6 +153,11 @@ static float search(FOCContext *foc, int pass, int maxpass, int xmin, int xmax, { int x, y; + /* The rounding in downscale() and of the window below can leave the coarse + * limit past the top level limit divided by two, so bound every level. */ + xmax = FFMIN(xmax, foc->haystack_frame[pass]->width - foc->needle_frame[pass]->width); + ymax = FFMIN(ymax, foc->haystack_frame[pass]->height - foc->needle_frame[pass]->height); + if (pass + 1 <= maxpass) { int sub_x, sub_y; search(foc, pass+1, maxpass, xmin>>1, (xmax+1)>>1, ymin>>1, (ymax+1)>>1, &sub_x, &sub_y, 2.0); -- 2.54.0 >From eeeac96edeaae57ea41675dc8c1558cab4d6cf52 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 13:03:43 +0100 Subject: [PATCH 3/3] avfilter/vf_find_rect: do not use the result of an empty coarse search A window with xmin > xmax or ymin > ymax only logs a warning and is searched anyway. The recursive call then finds nothing, returns the initial score of 2.0 and leaves sub_x and sub_y uninitialized, and the caller computes its refinement window from them. Refine only when the coarse search found a position. compare() returns at most 1.0, so a score below 2.0 means it did. Signed-off-by: lusoris <[email protected]> --- libavfilter/vf_find_rect.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c index 9b537115494..11241882fe1 100644 --- a/libavfilter/vf_find_rect.c +++ b/libavfilter/vf_find_rect.c @@ -160,11 +160,13 @@ static float search(FOCContext *foc, int pass, int maxpass, int xmin, int xmax, if (pass + 1 <= maxpass) { int sub_x, sub_y; - search(foc, pass+1, maxpass, xmin>>1, (xmax+1)>>1, ymin>>1, (ymax+1)>>1, &sub_x, &sub_y, 2.0); - xmin = FFMAX(xmin, 2*sub_x - 4); - xmax = FFMIN(xmax, 2*sub_x + 4); - ymin = FFMAX(ymin, 2*sub_y - 4); - ymax = FFMIN(ymax, 2*sub_y + 4); + /* An empty window finds nothing and leaves sub_x and sub_y unset. */ + if (search(foc, pass+1, maxpass, xmin>>1, (xmax+1)>>1, ymin>>1, (ymax+1)>>1, &sub_x, &sub_y, 2.0) < 2.0) { + xmin = FFMAX(xmin, 2*sub_x - 4); + xmax = FFMIN(xmax, 2*sub_x + 4); + ymin = FFMAX(ymin, 2*sub_y - 4); + ymax = FFMIN(ymax, 2*sub_y + 4); + } } for (y = ymin; y <= ymax; y++) { -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
