#208 faac: CVE-2017-9129 CVE-2017-9130

[Fabian Greffrath]

[Copied over from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=865909]

Hi,

the following vulnerabilities were published for faac.

CVE-2017-9129[0]:
| The wav_open_read function in frontend/input.c in Freeware Advanced
| Audio Coder (FAAC) 1.28 allows remote attackers to cause a denial of
| service (large loop) via a crafted wav file.

CVE-2017-9130[1]:
| The faacEncOpen function in libfaac/frame.c in Freeware Advanced Audio
| Coder (FAAC) 1.28 allows remote attackers to cause a denial of service
| (invalid memory read and application crash) via a crafted wav file.

If you fix the vulnerabilities please also make sure to include the
CVE (Common Vulnerabilities & Exposures) ids in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2017-9129
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-9129
[1] https://security-tracker.debian.org/tracker/CVE-2017-9130
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-9130
[2] https://www.exploit-db.com/exploits/42207/

[Fabian Greffrath]

1) I can confirm that CVE-2017-9129 has already been entirely fixed by this commit:

http://faac.cvs.sourceforge.net/viewvc/faac/faac/frontend/input.c?r1=1.16&r2=1.17&sortby=date

2) According to the API documentation, the faacEncOpen() function is used to "open and initialize an encoder instance". It takes the number of channels of the input data (numChannels) as its second arguent and allocates memory for a variable of type faacEncStruct. This struct is defined in libfaac/frame.h and contains several elements that are arrays of size MAX_CHANNELS, which in turn is defined to the value 64 in libfaac/coder.h.

Later on, this function iterates from 0 to numChannels to apply values to the elements in the arrays in this struct. However, it does never check that the value of numChannels does not exceed MAX_CHANNELS (in the crafted file at hand its value is 65281), which leads to the segmentation fault described in the CVE report. In the patch attached here, a NULL pointer is returned instead if numChannels exceeds MAX_CHANNELS.

The API documentation in docs/libfaac.html states that "If anything goes wrong NULL is returned." In fact, this never happens. So the second chunk of my patch is to prepare the frontend for this function to return a NULL pointer and fail gracefully.

The third chunk replaces hard-coded values of 64 with the MAX_CHANNELS macro.

Maybe it's time to release a 1.29 version now?

[Krzysztof Nikiel]

OK, thanks--it will help.
When I was looking into it, I found another problem with reading the wave format. I will try to commit the patches tomorrow.

I can tag the source as 1.29, but I don't know if I can actually release project files here. Possibly only project admins can do that.

[Krzysztof Nikiel]

Let's hope I commited it correctly -> closed

[Fabian Greffrath]

Did you mean to add the following check to frontend/input.c?

[Fabian Greffrath]

Rationale: Let's please not use hard-coded values if we in-fact mean the size of a specific object. Also, are you going to increase the version counter in configure.in, too?

[Krzysztof Nikiel]

Yes, I know '16' looks pretty bad.
Unfortunatelly typical wav files have such format size for some reason and it doesn't match any struct(WAVEFORMAT is 14, WAVEFORMATEX is 18).
It's weird.

[Krzysztof Nikiel]

I don't know why I thought I can't upload files, apparently I can.
FAAC 1.29 files released :D

[Fabian Greffrath]

Cool, congratulations for the new release!

Would you mind granting me a role in the project? I think sourceforge has e.g. "packager" which would fit perfectly. I currently have problems checking out the CVS sources as an anonymous user.

[Krzysztof Nikiel]

I'm pretty sure only admins can do that. I think we should contact Menno.

[Krzysztof Nikiel]

OK, it's done. Menno added me to admin group so I can do much more here.
I just added you to "Release Technicans", hopefully it works.

Also, the plan is to migrate the project to git repo.
I will try to make cvs->git transition.

As to CVS, it looks like you are in the developer group, I don't know why wouldn't you have developer access.
Hopefully I will figure out the whole group/permission SF thing.

[Fabian Greffrath]

Wow, great news! Thanks for taking care of that!

[Fabian Greffrath]

Argh, the release tarball contains the whole CVS directories. Next time, please package a CVS export, not a checkout, thanks!

[Krzysztof Nikiel]

Oops, maybe we should have some script to crate the package.
btw, can you actually add new files or delete them?