Consider releasing version 2.5.1 with ABI version 7

[mcatanzaro]

OpenH264 2.6.0 fixes this heap buffer overflow issue. Users are advised to upgrade to version 2.6.0. But this is not easy, because 2.6.0 increases the ABI version from 7 to 8, necessitating rebuilds of all dependent applications. This is not permitted in most Linux distributions; generally Linux distributors only do ABI version bumps in development releases (e.g. Fedora 42, freedesktop-sdk 25.08) and leave stable releases (e.g. Fedora 41, Fedora 40, freedesktop-sdk 24.08) on one ABI version forever.

A normal solution to resolve the security issue without an ABI bump is add a patch. This is generally not possible with OpenH264 for two reasons:

• The security advisory does not say which commit fixes the security issue. Would be nice to know which commit is required!
• Distributors shipping OpenH264 generally cannot patch it because they use OpenH264 solely to take advantage of Cisco's patent license. Using a non-Cisco build would defeat the point of using OpenH264. (Fedora is an exception to this; thank you Cisco for kindly hosting Fedora's builds!)

There are two easy solutions here:

• Release OpenH264 2.5.1 with the security fix and the previous ABI version 7, which would allow users to receive the security fix today instead of waiting for a new operating system or Flatpak runtime version.
• Say "whoops don't use version 2.6.0" and release a 2.6.1 with ABI version 7, because it looks like the ABI version bump was probably unnecessary. abidiff doesn't show any breaking changes between 2.5.1 and 2.6.0, so it's unclear why the ABI version was increased. (Of course, this downgrade would be inconvenient for anybody who is already using version 2.6.0, since it would require rebuilds of all dependent applications.)

Whether it's called 2.6.1 or 2.5.1, a new release with version 7 ABI would be very helpful to get this security fix out to users.

[yselkowitz]

My guess would be that 63db555 is the fix for the CVE, but as you say, that doesn't help much given the overall situation.

[BenzhengZhang]

Hi @mcatanzaro,thanks for you advises!!
First, here are the answers to your questions:

• This commit 63db555 fix the security issue.
• bump ABI version because of the api change involved by 33f7f48

If I choose to release openh264v2.5.1, we would just need to include the commit that fixes the vulnerabilities and create a new branch called openh264v2.5.1. Should I also provide a dynamic library through the release page, similar to v2.5.0, v2.6.0, etc.? I'm not sure how Fedora or other organizations build the openh264 binaries, so I have some questions about that.
Just out of curiosity, I noticed that Fedora's builds hosted by Cisco have stayed at version 2.4.1 for Fedora versions 38-42. Will the next build upgrade Fedora 38-41 to v2.5.1, and will Fedora 42 (the development release) upgrade to v2.6.0?

[Erick555]

Hi, @BenzhengZhang , the commit you mention in descriptions says This is disabled by default. Does it mean the 2.6.0 binaries published by cisco aren't affected by the api change and are compatible with 2.5.0?

[bbhtt]

abidiff is reporting no changes between 2.6.0 and 2.5.0

abidiff libopenh264-2.6.0-linux64.8.so libopenh264-2.5.0-linux64.7.so 

ELF SONAME changed
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

SONAME changed from 'libopenh264.so.8' to 'libopenh264.so.7'

[mcatanzaro]

I guess abidiff is either misconfigured or broken? Commit 33f7f48 changes the size of SEncParamExt, and this is exact same struct from the previous ABI break issue report #3564 so we know it will be a problem in practice. I think Benzheng was correct to bump the ABI version. (Any ideas what is wrong with abidiff?)

If I choose to release openh264v2.5.1, we would just need to include the commit that fixes the vulnerabilities and create a new branch called openh264v2.5.1. Should I also provide a dynamic library through the release page, similar to v2.5.0, v2.6.0, etc.?

Yes please. The library released by Cisco is what freedesktop-sdk users ultimately wind up downloading.

I'm not sure how Fedora or other organizations build the openh264 binaries, so I have some questions about that.

Fedora is special because you (Cisco) host our builds. We have downstream packaging here and we can do whatever we want there.

Other projects like freedesktop-sdk do not build OpenH264 themselves, and just download the binaries released by Cisco.

Just out of curiosity, I noticed that Fedora's builds hosted by Cisco have stayed at version 2.4.1 for Fedora versions 38-42.

Fedora decommissioned the tool for updating the OpenH264 repo and took several months to figure out what to do about this, so it wasn't possible to make any updates at all. But that problem is now fixed.

Will the next build upgrade Fedora 38-41 to v2.5.1, and will Fedora 42 (the development release) upgrade to v2.6.0?

Fedora rawhide will update to v2.6.0 (and rebuild all dependencies of OpenH264).

I would prefer not to update Fedora 42 to a new ABI version. We should certainly update Fedora F42 to the new v2.5.1, though.

For older versions of Fedora, we have a choice between updating or just adding the patch for commit 63db555. This is a tough decision to make. Maybe updating from 2.4 -> 2.5 will fix more than it breaks, or maybe it will introduce regressions. Probably just adding the patch would be safer.

[mcatanzaro]

I'm not sure how Fedora or other organizations build the openh264 binaries, so I have some questions about that.

The other thing worth mentioning is noopenh264 which is a stub library to allow Fedora, openSUSE, and freedesktop-sdk build applications and libraries that depend on OpenH264 without actually having the real OpenH264 available. (This library will need to find a new home because freedesktop-sdk is discontinuing its OpenH264 extension.)

[BenzhengZhang]

@mcatanzaro Thank you for your answers and knowledge sharing 👍

I'm also puzzled as to why changing the API structure doesn't break the ABI. However, from a practical standpoint and to avoid crash risks, it is more recommended that all users rebuild their applications, even if abidiff shows no ABI breakage. The master version and later versions of 2.6.0 will still be retained, and the shared version is 8. I will also make a v2.5.1 release, which will include the security fix and have a shared version number of 7.

I've pinned this issue to see if anyone has a better perspective or advice on it.

[bbhtt]

I'm also puzzled as to why changing the API structure doesn't break the ABI.

I've asked the libabigail maintainer about this but still waiting for an answer. Most likely it doesn't have access to the headers and debug data which we usually supply when comparing ABI.

Rebuilding everything is not an option always especially in case of Flatpak runtimes, where bi-directional ABI is maintained and a single runtime can be used by a couple of 1000 apps and runtime maintainers have no control over that.

Ideally new API that's disabled by default and requires an ABI bump should be done after releasing a version with the security fix with the old ABI. That makes it easier for everyone because we can just update a single package to release the fix.

I will also make a v2.5.1 release, which will include the security fix and have a shared version number of 7.

Thank your for this.

[bbhtt]

I'm also puzzled as to why changing the API structure doesn't break the ABI.

I've asked the libabigail maintainer about this but still waiting for an answer. Most likely it doesn't have access to the headers and debug data which we usually supply when comparing ABI.

According to the libabigail maintainer this is because the ELFs are stripped and have no debuginfo. To quote:

so no analysis of /types/ can take place.

With debuginfo we can indeed see the diff.

ABI diff

abidiff libopenh264.so.2.5.0 libopenh264.so.2.6.0
Functions changes summary: 0 Removed, 3 Changed (455 filtered out), 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

3 functions with some indirect sub-type change:

  [C] 'function int32_t WelsCreateSVCEncoder(ISVCEncoder**)' at welsEncoderExt.cpp:1372:1 has some indirect sub-type changes:
    parameter 1 of type 'ISVCEncoder**' has sub-type changes:
      in pointed to type 'ISVCEncoder*':
        in pointed to type 'class ISVCEncoder' at codec_api.h:272:1:
          type size hasn't changed
          2 member function changes (10 filtered):
            'method virtual int ISVCEncoder::InitializeExt(const SEncParamExt*)' has some sub-type changes:
              parameter 1 of type 'const SEncParamExt*' has sub-type changes:
                in pointed to type 'const SEncParamExt':
                  in unqualified underlying type 'typedef SEncParamExt' at codec_app_def.h:598:1:
                    underlying type 'struct TagEncParamExt' at codec_app_def.h:540:1 changed:
                      type size changed from 7360 to 7392 (in bits)
                      3 data member insertions:
                        'bool bPsnrY', at offset 7360 (in bits) at codec_app_def.h:595:1
                        'bool bPsnrU', at offset 7368 (in bits) at codec_app_def.h:596:1
                        'bool bPsnrV', at offset 7376 (in bits) at codec_app_def.h:597:1
            'method virtual int ISVCEncoder::EncodeFrame(const SSourcePicture*, SFrameBSInfo*)' has some sub-type changes:
              parameter 1 of type 'const SSourcePicture*' has sub-type changes:
                in pointed to type 'const SSourcePicture':
                  in unqualified underlying type 'typedef SSourcePicture' at codec_app_def.h:669:1:
                    underlying type 'struct Source_Picture_s' at codec_app_def.h:655:1 changed:
                      type size changed from 576 to 640 (in bits)
                      3 data member insertions:
                        'bool bPsnrY', at offset 576 (in bits) at codec_app_def.h:666:1
                        'bool bPsnrU', at offset 584 (in bits) at codec_app_def.h:667:1
                        'bool bPsnrV', at offset 592 (in bits) at codec_app_def.h:668:1
              parameter 2 of type 'SFrameBSInfo*' has sub-type changes:
                in pointed to type 'typedef SFrameBSInfo' at codec_app_def.h:654:1:
                  underlying type 'struct SFrameBSInfo' at codec_app_def.h:643:1 changed:
                    type size changed from 41152 to 57536 (in bits)
                    4 data member changes:
                      type of 'SLayerBSInfo sLayerInfo[128]' changed:
                        array element type 'typedef SLayerBSInfo' changed:
                          underlying type 'struct SLayerBSInfo' at codec_app_def.h:623:1 changed:
                            type size changed from 320 to 448 (in bits)
                            1 data member insertion:
                              'float rPsnr[3]', at offset 320 (in bits) at codec_app_def.h:641:1
                        array type size changed from 40960 to 57344
                      'EVideoFrameType eFrameType' offset changed from 41024 to 57408 (in bits) (by +16384 bits)
                      'int iFrameSizeInBytes' offset changed from 41056 to 57440 (in bits) (by +16384 bits)
                      'long long int uiTimeStamp' offset changed from 41088 to 57472 (in bits) (by +16384 bits)

  [C] 'function int32_t WelsEnc::AcquireLayersNals(WelsEnc::sWelsEncCtx**, WelsEnc::SWelsSvcCodingParam*, int32_t*, int32_t*)' at encoder_ext.cpp:749:1 has some indirect sub-type changes:
    parameter 1 of type 'WelsEnc::sWelsEncCtx**' has sub-type changes:
      in pointed to type 'WelsEnc::sWelsEncCtx*':
        in pointed to type 'typedef WelsEnc::sWelsEncCtx' at rc.h:53:1:
          underlying type 'struct WelsEnc::TagWelsEncCtx' at encoder_context.h:116:1 changed:
            type size hasn't changed
            1 data member changes (5 filtered):
              type of 'SWelsSvcCodingParam* pSvcParam' changed:
                in pointed to type 'typedef WelsEnc::SWelsSvcCodingParam' at param_svc.h:538:1:
                  underlying type 'struct WelsEnc::TagWelsSvcCodingParam' at param_svc.h:106:1 changed:
                    type size hasn't changed
                    1 base class change:
                      'struct TagEncParamExt' at codec_app_def.h:540:1 changed:
                        details were reported earlier
                    3 data member changes:
                      'SSpatialLayerInternal sDependencyLayers[4]' offset changed from 7360 to 7392 (in bits) (by +32 bits)
                      'uint32_t uiGopSize' offset changed from 9536 to 9568 (in bits) (by +32 bits)
                      'struct {int32_t iLeft; int32_t iTop; int32_t iWidth; int32_t iHeight;} SUsedPicRect' offset changed from 9568 to 9600 (in bits) (by +32 bits)

  [C] 'method void WelsEnc::CWelsH264SVCEncoder::DumpSrcPicture(const SSourcePicture*, const int)' at welsEncoderExt.cpp:1311:1 has some indirect sub-type changes:
    implicit parameter 0 of type 'WelsEnc::CWelsH264SVCEncoder*' has sub-type changes:
      in pointed to type 'class WelsEnc::CWelsH264SVCEncoder' at welsEncoderExt.h:59:1:
        type size hasn't changed
        1 base class change:
          'class ISVCEncoder' at codec_api.h:272:1 changed:
            details were reported earlier
        no member function changes (13 filtered);
        no data member change (1 filtered);

[BenzhengZhang]

branch: https://github.com/cisco/openh264/tree/openh264v2.5.1
Binaries are in the works...