Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't update sps if they are only repeated #372

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

coldtobi
Copy link

@coldtobi coldtobi commented Jan 13, 2023

This is an attempt to improve the mitigations from #365 and #366.

The mitigations in the PR assumes that sps with different pointers are incompatible; however, when new sps are encountered, always a new sps object is created, leading to an over-rejection of sps, as they might just have been repeated with compatible information. I guess this over-rejection might cause that images cannot be decoded, which would otherwise be decode-able.

It picks up the idea from #345 (comment):

One way would be just to look at the pointers of the SPS (fast and easy, but may reject more than required), or investigate if the SPS used for the image generations are "compatible".

This changes do exactly this: It (very conservativly -- as in "use new one if in doubt") checks if the old and new sps have identical information -- except the reference picture set, which I believe is supposed to be updated by new sps'). If they are basically identical, the old sps will be used instead of the new one, (of course, reference image set is updated from the new one)

I'm using standalone operator== and helper functions to avoid changing ABI of the library; if an ABI bump would be done, of course this should go to the respective classes.

I've tested the patch with several videos, they still play fine.

@farindk I'd really appreciate to receive your feedback; the reason is that I want to fix the many open CVE's for the package as it is currently in Debian and other distributions…

--
Cheers,
tobi

This is an attempt to improve the mitigations from strukturag#365 and strukturag#366 and picks up an idea I described at strukturag#345:

> One way would be just to look at the pointers of the SPS (fast and easy, but
> may reject more than required), or investigate if the SPS used for the image
> generations are "compatible".

This changes do exactly this: It (very conservativly) checks if the old and new sps have
identical information -- except the reference picture set, which I believe is supposed
to be updated by new sps'). If they are basically identical, the old sps will be
used instead of the new one, (of course, reference image set is updated from the new one)

I'm using standalone operator== and helper functions to avoid changing ABI of the library;
if an ABI bump would be done, of course this should go to the respective classes.
@farindk
Copy link
Contributor

farindk commented Jan 24, 2023

The CVEs have been fixed in another way, but I leave this open as it would be nice to have a check like this that can detect when there was an illegal SPS switch in the input stream.

@coldtobi
Copy link
Author

thanks @farindk for your work on all of those issue. Do plan to cut a new release soon? Asking, because Debian is entering freeze in around two weeks and it would be nice to have your official version in the next stable…

@farindk
Copy link
Contributor

farindk commented Jan 27, 2023

@coldtobi Yes, I've just released v1.0.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants