mirror of
https://github.com/FFmpeg/FFmpeg.git
synced 2026-11-06 05:17:42 +00:00
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 what the cast produces depends on the target: measured on AArch64, where it saturates, "+1e20" happened to be rejected downstream while "-1e20" was accepted. - 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
This commit is contained in:
1 parent
21e6c1f57d
commit
e2740a3e0d
3 files changed
+33
-3
No files matched your search
+11
-2
@@ -505,11 +505,20 @@ static int set_string_number(void *obj, void *target_obj, const AVOption *o, con
|
||||
}
|
||||
}
|
||||
if (type == AV_OPT_TYPE_FLAGS && dst) {
|
||||
unsigned int operand;
|
||||
/* Validate the operand of a '+' or '-' update and let
|
||||
* write_number() convert it: casting 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, &operand, d, 1, 1, 0)) < 0)
|
||||
return ret;
|
||||
intnum = *(unsigned int*)dst;
|
||||
if (cmd == '+')
|
||||
d = intnum | (int64_t)d;
|
||||
d = intnum | operand;
|
||||
else if (cmd == '-')
|
||||
d = intnum &~(int64_t)d;
|
||||
d = intnum &~operand;
|
||||
}
|
||||
|
||||
if ((ret = write_number(obj, o, dst, d, 1, 1, 0)) < 0)
|
||||
|
||||
@@ -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",
|
||||
@@ -549,7 +553,9 @@ int main(void)
|
||||
av_log_set_level(AV_LOG_QUIET);
|
||||
|
||||
for (i=0; i < FF_ARRAY_ELEMS(options); i++) {
|
||||
int silence_log = !strcmp(options[i], "rational=-1/0"); // inf formatting differs between platforms
|
||||
// inf formatting differs between platforms, and so may %f of 1e20
|
||||
int silence_log = !strcmp(options[i], "rational=-1/0") ||
|
||||
strstr(options[i], "100000000000000000000");
|
||||
av_log(&test_ctx, AV_LOG_DEBUG, "Setting options string '%s'\n", options[i]);
|
||||
if (silence_log)
|
||||
av_log_set_callback(NULL);
|
||||
@@ -559,6 +565,8 @@ int main(void)
|
||||
printf("OK '%s'\n", options[i]);
|
||||
av_log_set_callback(log_callback_help);
|
||||
}
|
||||
// a rejected +x/-x flags update must leave the value untouched
|
||||
printf("flags=%d\n", test_ctx.flags);
|
||||
av_opt_free(&test_ctx);
|
||||
}
|
||||
|
||||
|
||||
@@ -269,6 +269,18 @@ 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'
|
||||
Value 1.500000 for parameter 'flags' is not a valid set of 32bit integer flags
|
||||
Error 'flags=+1.5'
|
||||
Setting options string 'flags=-1.5'
|
||||
Setting entry with key 'flags' to value '-1.5'
|
||||
Value 1.500000 for parameter 'flags' is not a valid set of 32bit integer flags
|
||||
Error 'flags=-1.5'
|
||||
Setting options string 'flags=+100000000000000000000'
|
||||
Error 'flags=+100000000000000000000'
|
||||
Setting options string 'flags=-100000000000000000000'
|
||||
Error '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'
|
||||
@@ -487,6 +499,7 @@ Setting options string 'array_int=2147483648'
|
||||
Setting entry with key 'array_int' to value '2147483648'
|
||||
Value 2147483648.000000 for parameter 'array_int' out of range [0 - 2.14748e+09]
|
||||
Error 'array_int=2147483648'
|
||||
flags=5
|
||||
|
||||
Testing av_opt_set_from_string()
|
||||
Setting options string ''
|
||||
|
||||
Reference in new issue
Block a user