obs-studio icon indicating copy to clipboard operation
obs-studio copied to clipboard

mac-avcapture: crash on calling `obs_source_get_properties`

Open dnaka91 opened this issue 3 years ago • 3 comments

Operating System Info

macOS 12

Other OS

No response

OBS Studio Version

Other

OBS Studio Version (Other)

Latest master (9e15114750e12100db31b7f0d9f6b6050c807237)

OBS Studio Log URL

https://obsproject.com/logs/Kk4QAfK3apRQlY9p

OBS Studio Crash Log URL

https://pastebin.com/SsSMiA2i

Expected Behavior

I'm building a plugin for OBS, and try to iterate over all the properties of all input sources.

So I get the list of available inputs with obs_enum_input_types. Then for each input, I try to get the properties with obs_get_source_properties.

This should not crash and just return the properties pointer.

Current Behavior

When getting the properties for the av_capture_input source (which comes from mac-avcapture), the application crashes.

A quick look at the source code revealed that obs_get_source_properties gets the obs_source_info and then calls info->get_properties(NULL). But in the mac-avcapture plugin, it directly casts the parameter to another struct and uses it, hence not properly handling a null pointer.

Steps to Reproduce

  1. Call obs_get_source_properties with av_capture_input as source ID parameter.
  2. OBS crashes.

Anything else we should know?

No response

dnaka91 avatar Aug 10 '22 09:08 dnaka91

Same issue happens with the screen_capture input from the mac-capture plugin,

dnaka91 avatar Aug 10 '22 09:08 dnaka91

Continued to the filters and it seems the vst_filter from the obs-vst plugin has the same problem.

dnaka91 avatar Aug 10 '22 09:08 dnaka91

And now with the last batch, the transitions, it seems the wipe_transition of obs-transitions crashes as well.

dnaka91 avatar Aug 10 '22 09:08 dnaka91

I'd guess that's because the only part of our codebase that calls these functions with NULL values is the init code for audio sources (so it doesn't affect these plugins). The other parts of our code that call get_properties actually pass data (either source->context.settings or source->context.data, source->info.type_data so this issue never occurred so far.

mac-capture actually requires valid data to be passed in its current form, so iterating over the properties without source data won't work there out of the box anyway.

I guess the use case is not too obscure, but it's not fully supported by the code base right now (and it works "by accident" as lots of sources actually don't need any source data when getting their properties).

@RytoEX, @WizardCM any idea what to do about this? Do we actually want to allow passing NULL here (which would require a pass over all existing sources ensuring correct handling of missing source data)?

PatTheMav avatar Aug 24 '22 17:08 PatTheMav

@RytoEX, @WizardCM any idea what to do about this? Do we actually want to allow passing NULL here (which would require a pass over all existing sources ensuring correct handling of missing source data)?

@notr1ch might have opinions on this.

RytoEX avatar Aug 24 '22 18:08 RytoEX

In this instance it does look like the various get_properties function pointers of sources are indeed faulty. The documentation states that calling obs_get_source_properties with a source ID should function. It seems the possibility of NULL source data has been overlooked in a lot of sources and they should probably be fixed to return defaults instead of crashing.

notr1ch avatar Aug 24 '22 20:08 notr1ch