PR #25555 opened by Alexkane URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25555 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25555.patch
set_string_number() parses the operand of a "+x" or "-x" flags update as a double and casts it to int64_t to OR it into, or mask it out of, the current flags. Only the combined value then reaches the range check in write_number(), so the operand itself was never validated: - A value outside the int64_t range, or inf, makes the conversion undefined behaviour (C17 6.3.1.4). UBSan reports it for av_opt_set(ctx, "flags", "+77777777777777777777", 0), and the result differs between architectures: on AArch64 the conversion saturates and "+1e20" happens to be rejected downstream while "-1e20" is accepted, on x86 both become INT64_MIN. - A fractional operand is silently truncated, so "+1.5" and "-1.5" are accepted although "1.5" is refused as not being a valid set of flags. Run the existing flags range check on the operand before the cast, so an invalid operand returns AVERROR(ERANGE) exactly like an invalid absolute value, as av_opt_set() documents, and add the four cases to fate-opt. Reported by FuzzAnything-ORG with a UBSan trace. Fixes #22389 >From 47dbf2163f2f06c103ccf82d018cf77f0835948b Mon Sep 17 00:00:00 2001 From: Alexkane <[email protected]> Date: Tue, 27 Oct 2026 23:16:49 -0400 Subject: [PATCH] avutil/opt: validate the operand of a flags update before casting it set_string_number() parses the operand of a "+x" or "-x" flags update as a double and casts it to int64_t to OR it into, or mask it out of, the current flags. Only the combined value then reaches the range check in write_number(), so the operand itself was never validated: - A value outside the int64_t range, or inf, makes the conversion undefined behaviour (C17 6.3.1.4). UBSan reports it for av_opt_set(ctx, "flags", "+77777777777777777777", 0), and the result differs between architectures: on AArch64 the conversion saturates and "+1e20" happens to be rejected downstream while "-1e20" is accepted, on x86 both become INT64_MIN. - A fractional operand is silently truncated, so "+1.5" and "-1.5" are accepted although "1.5" is refused as not being a valid set of flags. Run the existing flags range check on the operand before the cast, so an invalid operand returns AVERROR(ERANGE) exactly like an invalid absolute value, as av_opt_set() documents, and add the four cases to fate-opt. Reported by FuzzAnything-ORG with a UBSan trace. Fixes #22389 --- libavutil/opt.c | 7 +++++++ libavutil/tests/opt.c | 4 ++++ tests/ref/fate/opt | 13 +++++++++++++ 3 files changed, 24 insertions(+) diff --git a/libavutil/opt.c b/libavutil/opt.c index 0f69e04b3b5..d69851a3e81 100644 --- a/libavutil/opt.c +++ b/libavutil/opt.c @@ -505,6 +505,13 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con } } if (type == AV_OPT_TYPE_FLAGS && dst) { + /* Validate the operand of a '+' or '-' update before it is cast: + * converting a double outside the int64_t range (or inf/nan) to + * int64_t is undefined behaviour, and the cast would also drop a + * fractional part that write_number() is meant to reject, so + * "+1.5" used to be accepted while "1.5" is refused. */ + if (cmd && (ret = write_number(obj, o, NULL, d, 1, 1, 0)) < 0) + return ret; intnum = *(unsigned int*)dst; if (cmd == '+') d = intnum | (int64_t)d; diff --git a/libavutil/tests/opt.c b/libavutil/tests/opt.c index 1f35d3dcb73..bafe14020d6 100644 --- a/libavutil/tests/opt.c +++ b/libavutil/tests/opt.c @@ -479,6 +479,10 @@ int main(void) "toggle=100", "toggle==1", "flags=+mu-lame : num=42: toggle=0", + "flags=+1.5", + "flags=-1.5", + "flags=+100000000000000000000", + "flags=-100000000000000000000", "num=42 : string=blahblah", "rational=0 : rational=1/2 : rational=1/-1", "rational=-1/0", diff --git a/tests/ref/fate/opt b/tests/ref/fate/opt index 90a7ba85e95..07b7d55682f 100644 --- a/tests/ref/fate/opt +++ b/tests/ref/fate/opt @@ -269,6 +269,19 @@ Setting entry with key 'flags' to value '+mu-lame' Setting entry with key 'num' to value '42' Setting entry with key 'toggle' to value '0' OK 'flags=+mu-lame : num=42: toggle=0' +Setting options string 'flags=+1.5' +Setting entry with key 'flags' to value '+1.5' +OK 'flags=+1.5' +Setting options string 'flags=-1.5' +Setting entry with key 'flags' to value '-1.5' +OK 'flags=-1.5' +Setting options string 'flags=+100000000000000000000' +Setting entry with key 'flags' to value '+100000000000000000000' +Value 9223372036854775808.000000 for parameter 'flags' is not a valid set of 32bit integer flags +Error 'flags=+100000000000000000000' +Setting options string 'flags=-100000000000000000000' +Setting entry with key 'flags' to value '-100000000000000000000' +OK 'flags=-100000000000000000000' Setting options string 'num=42 : string=blahblah' Setting entry with key 'num' to value '42' Setting entry with key 'string' to value 'blahblah' -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
