set_string_bool() overwrites memory.

[Wodzu]

Summary of the bug:

I am playing with ffmpeg and I wrote a custom filter based on the documentation. I've noticed erroneous behavior during function chain:
graph_opts_apply->filter_opt_apply->filter_opt_apply->av_opt_set->set_string_bool for my bool option.

Here is the definition of my context and options:

typedef struct FoobarContext {
    const AVClass *class;
    void* some_pointer;
    char* opt_1;
    char* opt_2;
    char* opt_3;
    char* opt_4;
    char* opt_5;
    float opt_6;
    float opt_7;
    float opt_8;
    bool opt_9;
    int opt_10;
    bool opt_11;
    bool opt_12;
    bool opt_13;
    bool opt_14;
} FoobarContext;


#define OFFSET(x) offsetof(FoobarContext, x)
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
static const AVOption foobar_options[] = {
    { "opt_1", "test", OFFSET(opt_1), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, FLAGS},
    { "opt_2",  "test",  OFFSET(opt_2), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, FLAGS},
    { "opt_3",  "test",  OFFSET(opt_3), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, FLAGS},
    { "opt_4",  "test",  OFFSET(opt_4), AV_OPT_TYPE_STRING, {.str = "" }, 0, 0, FLAGS},
    { "opt_5",  "test",  OFFSET(opt_5), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, FLAGS},
    { "opt_6",  "test",  OFFSET(opt_6), AV_OPT_TYPE_FLOAT, {.dbl = 0.01 }, 0, 1, FLAGS},
    { "opt_7",  "test",  OFFSET(opt_7), AV_OPT_TYPE_FLOAT, {.dbl = 0.01 }, 0, 1, FLAGS},
    { "opt_8",  "test",  OFFSET(opt_8), AV_OPT_TYPE_FLOAT, {.dbl = 0.45 }, 0, 1, FLAGS},
    { "opt_9",  "test",  OFFSET(opt_9), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS},
    { "opt_10",  "test",  OFFSET(opt_10), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 10, FLAGS},
    { "opt_11",  "test",  OFFSET(opt_11), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS},
    { "opt_12",  "test",  OFFSET(opt_12), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS},
    { "opt_13",  "test",  OFFSET(opt_13), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS},
    { "opt_14",  "test",  OFFSET(opt_14), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS},
    { NULL }
};

As you can see, from opt_11 until opt_14 we have bool options. OFFSET macro is creating offset on one byte boundary for these fields. So for opt_11 the offset is 76, for opt_12 offset is 77, for opt_13 offset is 78 and (unsurprisingly at that point) for opt_14 offset is 79.

The problem is that function set_string_bool(void *obj, const AVOption *o, const char *val, int *dst)
is setting value of dst which is pointer to int. int is 4 byte long so it is overwriting default values of other 3 options as well.

How to reproduce:
Either find existing filter that contain grouped bool options or implement a simple pass through filter with the options above.

Then run command like this:

% ffmpeg -i -i http://samples.ffmpeg.org/image-samples/lena.pnm "foobar=opt_11=y" foobar.png
ffmpeg version N-110411-gaf8be7bf43 Copyright (c) 2000-2023 the FFmpeg developers
  built with gcc 11 (Ubuntu 11.3.0-1ubuntu1~22.04.1)

Guides to fix: change dst function parameter to uint8_t* , change "int n" variable to be type of bool.

[Cigaes]

AV_OPT_TYPE_INT must point to int fields.

[Elon Musk]

BOOL avoption type is not using bool type at all.
This is because bool is not available in C99.
So fix your code to replace all bool types with int.
Bunch of examples in other filters in libavfilter confirm this.
Why you think otherwise is prize worth a lot.

[Wodzu]

Replying to Cigaes:

AV_OPT_TYPE_INT must point to int fields.

I think you've meant AV_OPT_TYPE_BOOL?

[Wodzu]

Replying to Elon Musk:

Why you think otherwise is prize worth a lot.

Why? because it does not make sense and it is undocumented and confusing, that is way. Bool is available in C99. Not only your entire comment is unnecessary as Cigaes already explained but you also trying to be a di*k for no reason.

[Elon Musk]

The only d*k person here is you.
When trying to add bool variable to random code in FFmpeg, I get compilation error, so one more mystery to add to resolve this nonsense bug report.

[Wodzu]

Replying to Elon Musk:

The only d*k person here is you.

You are acting like a child.

When trying to add bool variable to random code in FFmpeg, I get compilation error, so one more mystery to add to resolve this nonsense bug report.

Yes, try to add more random stuff, you will have more mysteries to solve. Maybe that will keep you from posting on Internet.

[Balling]

The following C99 features must not be used anywhere in the codebase:

variable-length arrays;
complex numbers;
mixed statements and declarations.

https://ffmpeg.org/developer.html#Coding-Rules-1

There is no prohibition. What is the point to use bool though? Just use 8 byte type IF YOU are really set on it, BOOL IS NOT 1 bit, okay! But really everyone just uses slightly bigger "unsigned", that is just unsigned int...

[Wodzu]

Replying to Balling:

Hi, thank you for the comment. In my opinion the purpose of using different types (including bool) is to better express your design, thus to write better code at the end. This faulty bug that I've created is a good example why a given type should represent as closely as possible the meaning of variable that it is used for. If it would, I wouldn't be confused and waste peoples time by issuing this"bug" report. But I understand, that ffmpeg probably was created when bool was just added to the C99 standard hence its representation by four bytes.

Cheers!

[Balling]

In my opinion the purpose of using different types (including bool) is to better express your design, thus to write better code at the end

Unless you have many such bools and use 1 bit per it and special CPU instructions this will be slower. In particular using 1 byte instead of 4 is slower.