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

Done: /src/libaudiofile needs to be updated to prevent security problems #160

Open
filochard opened this issue Dec 9, 2023 · 15 comments
Open

Comments

@filochard
Copy link

the /src/libaudiofile needs to be updated to prevent security problems

the bell has been rung here :
OpenCPN/OpenCPN#3216 (comment)

since the libaudio source has been imported in weatherfax_pi some CVE have appeared
Some patches have been created for this problem

here are the source and the patches for debian :
http://deb.debian.org/debian/pool/main/a/audiofile/audiofile_0.3.6.orig.tar.gz
http://deb.debian.org/debian/pool/main/a/audiofile/audiofile_0.3.6-6.debian.tar.xz

NB Some of these patches may have already been applied to the source used by weatherfax_pi

@rgleason
Copy link
Owner

portaudio.dll v19 (no yet API 2.0) quite...
https://github.com/PortAudio/portaudio/wiki/V19ReleasePlan

@rgleason
Copy link
Owner

@filochard
Copy link
Author

portaudio.dll v19 (no yet API 2.0) quite... https://github.com/PortAudio/portaudio/wiki/V19ReleasePlan

Hi Rick
portaudio is not the culprit I was talking about

I talked about libaudiofile which is not in opencpn-libs
but which is in weatherfax_pi tree
in weatherfax_pi/src/libaudiofile itself and in its sub directories
weatherfax_pi/src/libaudiofile/alac
and weatherfax_pi/src/libaudiofile/modules
.... nearly one hundred files !!!

if you look at the content of http://deb.debian.org/debian/pool/main/a/audiofile/audiofile_0.3.6-6.debian.tar.xz
you will have all the patches that have been done for several reasons (CVE particularly)
Some of them have probably be applied to the source you use : you'll have to compare your source and the patches
NB those patches need to be applied incrementally in the order they are numbered !

On my side I have updated the rpms of audiofile for Mageia (we don't use your bundled version) and a friend of mine is gonna test weatherfax built on it with is own radio station...

@rgleason
Copy link
Owner

Ok, so the problem is to update libaudio to current standards by applying patches in the righ order.
However portaudio for windows should also be updated for windows.

@filochard
Copy link
Author

NB
I don't know where the original source of audiofile that is used here comes from...
It is very different from the one used by Debian or Fedora
The patches from Debian can't be applied
The patch gcc6 should modify /libaudiofile/modules/SimpleModule.h line 126 in the Debian source
but the file here in wetherfax_pi is different from Debian source : a strange line 127 has been added and will prevent the patch to be applied

This will not be a piece of cake to provide an updated source for audiofile inside weatherfax_pi with its actual source

I think that the best to do will be to start from the orig source from debian and then to patch it with the patches from debian
http://deb.debian.org/debian/pool/main/a/audiofile/audiofile_0.3.6.orig.tar.gz
http://deb.debian.org/debian/pool/main/a/audiofile/audiofile_0.3.6-6.debian.tar.xz

@rgleason
Copy link
Owner

This will not be a piece of cake to provide an updated source for audiofile inside weatherfax_pi with its actual source
Agreed. I am not able to do this, hopefully we can find someone who knows about this.

@filochard
Copy link
Author

Hi Rick
if it can be useful for you I can send you the audiofile source with all the patches applied
You will only have to compare each modified part with the ones you provide here, and commit the diff
Let me know : I will attach the tar.gz to a next comment

@filochard
Copy link
Author

Hi Rick
here is a tarball of the parts of libaudiofile that have been corrected during last years because of CVE or because of bugs preventing to compile them correctly
You may compare them with what is in your /src/libaudiofile tree and apply the diff to each of them

corrected-libaudiofile.tar.gz

Here the list of the parts to modify :
/src/libaudiofile/modules/ALAC.cpp
/src/libaudiofile/modules/BlockCodec.cpp
/src/libaudiofile/modules/IMA.cpp
/src/libaudiofile/modules/ModuleState.cpp
/src/libaudiofile/modules/MSADPCM.cpp
/src/libaudiofile/modules/SimpleModule.cpp
/src/libaudiofile/modules/SimpleModule.h
/src/libaudiofile/NeXT.cpp
/src/libaudiofile/WAVE.cpp

That wouldn't be so difficult to commit

Hope this will help to have a more solid plugin (without security risks)

@rgleason
Copy link
Owner

rgleason commented Jan 23, 2024

@filochard Thank you, I will take a look at it shortly. Hope I am up to the task.

@rgleason
Copy link
Owner

I extracted and copied your list using winmerge. I found that IMA.cpp and MSADPCM.cpp are identical, but copied them anyway. WE will see if this works. Thanks.

@filochard
Copy link
Author

I extracted and copied your list using winmerge. I found that IMA.cpp and MSADPCM.cpp are identical, but copied them anyway. WE will see if this works. Thanks.

Hi Rick
That means that the version of libaudiofile used here had already been patched for the problems of those 2 parts of the source
The others parts were patched after this version was used here
That's why when building rpms I prefer using our own libraries than the bundled ones

Nevertheless this feedback will be useful for you, and for OpenCPN ;-)

@rgleason
Copy link
Owner

@filochard I built it for windows and tried just the internet functions. Can you test the libaudio?
I pushed it up to cloudsmith and the tarballs should be here once it all builds and is deployed to cloudsmith.

https://cloudsmith.io/~opencpn/repos/weatherfax-prod/packages/?q=1.9.111.1+tarball

After you test it as OK I will push up to PIM. Thanks.

@rgleason rgleason changed the title the /src/libaudiofile needs to be updated to prevent security problems Done: /src/libaudiofile needs to be updated to prevent security problems Jan 23, 2024
@rgleason
Copy link
Owner

@filochard Is this fixed now that we updated libaudiofile?
Bug: Flatpak Receiving sound #159

@rgleason
Copy link
Owner

@filochard can we close this now? Also close Bug: Flatpak Receiving sound #159 ?

@filochard
Copy link
Author

Hi Rick
I can't test flatpak at home (no radio receiver connected to my computer)
(you may know that I continue to create and use legacy package, adapting the sources for this purpose)
I can only say that with our weatherfax_pi rpm built upon our own patched libaudiofile (instead of the bundled one) weatherfax was working perfectly
There's no reason it would be different for flatpak

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

No branches or pull requests

2 participants