Uploading new modules carries some risks - CVE-2024-53564

If modules are expected to be in certain formats, tar.gz, tgz.gpg, etc…then the actual upload form shouldn’t allow every file format under the sun to be accepted and put on the server before realizing “oh wrong file type”. Even then it could at least be removed from the system so it’s just not sitting there to be exploited in some way. If a .sh file shouldn’t be accepted, then don’t let the user actually upload a .sh file to the system, show an error but still leave it on the system. Why aren’t these bad formats removed once found to be invalid for this function?

Opinion from a third-party module developer…

Nothing should be done to the GUI or CLI that makes it more difficult to install legitimate third-party modules. Upload from web, or fwconsole ma downloadinstall <url> or whatever, should be allowed and without warnings/obstructions.

More verification of filetypes, contents, signature files, etc. makes sense though.

3 Likes

That would still work. CLI change is not part of the proposal. OP and CVE are regarding uploading new modules via the GUI.

A y/n boolean prompt like you see when running other package manager updates that are about to modify executable files on your system could help users make safer, more informed choices.

Another option might be to include another toggle in the admin section of the GUI. Or to break it out via User/Group permissions in the GUI. But these still leave control in the GUI.

What aren’t there checks being done on the client side? Let’s not break a key feature of the PBX because simple security and validation methods can’t be used properly.

If System is enabled, and you can inject its parameters, you can use that to upload files, albeit possibly in small pieces, not in a big chunk.

Just out of curiosity, how would they upload the file via System() calls in the dialplan? You mean download a file?

Does it take a lot of effort to do this even if it’s not declared in the CVE?

A one line text file is trivial: just run echo and redirect the output. You can run multiple echoes, one for each line. For binary, you can use base64 utilities, or uudecode.

I have not idea what you are referring to

System (echo XXXXXX | base64 -d > testupload)

where XXXXXX is the result of running base64, with no options, or with -w 0, on the rogue file.

Looking at the history of the “live dangerously” setting, the assumption is that, if you can install new dialplan, you have complete control of the asterisk user.

1 Like

Sure, that can happen too. This is one of those things, even with the CVE, that requires the system to already be compromised. The attacker either got SSH access or got into the GUI some how. At this point, the attacker is authorized.

All this CVE shows is that the module upload has no client side validation of the files being uploaded. The server validation stops if the file isn’t the proper format but it doesn’t actually remove the file. It just sits there. These are things that shouldn’t be happening. I’m not even sure this CVE should be scored as high as it was.

Interestingly, that Def Con 31 presentation touched on how the flawed the Admin/User permissions were and how they weren’t separated enough and that someone with user privileges could worm their way into admin privileges.

Also, at the end of the day even if it was a signed module being uploaded…they can do bad things in the install process anyways.

So all these examples and scenarios all have the same base issue…if the SSH or GUI access is compromised, bad things can happen. Without either level of access, these bad things can’t happen.

I mean with the lack of code review and other issues with the process, a bad actor could put malicious code in the Ring Group module. Between almost no code review and questionable (if any) QA…bad things can still happen.

1 Like

I think they were referring not to how Sangoma’s reporting process was done, just the fact it was largely ignored. Like ours when we were ignored when reporting through support. Referring to “well that’s before and its like this now”, is not confidence to reports being ignored.

There’s some checks on the file extension but no pensive reflections on magic bytes, malware introspection, etc., before exec()'ing. For example, uploaded or downloaded, if it ends in zip, then Module Admin will happily feed the file to the unzip binary, which can harbor its own demons:pray: ZipArchive :pray:

Agreed. The 8.8 should be at most 6.5 per my scoring CVSS:3.1/AV:L/AC:L/PR:H/UI:R/S:U/C:H/I:H/A:H (which is still annoyingly high.)

This particular issue has probably been present for over a decade, but work is being done to improve the process, especially when it comes to security reporting, which saw a number of leaps forward in 2024, as discussed earlier in this post. For example, a similar path traversal upload issue in OSS EPM was handled according to policy in the fall.

Cutting out this upload option completely from the GUI requires a working Delete or Backspace key. :wink:

1 Like

Not the proper solution to this.

2 Likes

This is the problem with not knowing the project or the history of the project. In fact all of the code that went in to FreePBX had at minimum 4 sets of eyes that looked at it. Every PR went through a review process. The modules then went through a QA process as it sat in edge with a schedule cycle of releasing within a week. This was all heavily documented along with uniformity standards, code standards, processes etc internally in the wiki. I spent several years building these processes and working with teams and the community to improve them. Many of these processes were discarded because the people enforcing them left. Unfortunately some folks need to be micromanaged and with nobody with the skills, knowledge or motivation to do so things just fall apart. Realistically they should have handed the keys to someone like @jcolp but they would probably have to double his pay and he isn’t really a FreePBX guy. I would dare say he doesn’t want to be. At the end of the day without someone who cares about standards, quality and the power to enforce both most people apparently will do the bare minimum required to maintain employment

2 Likes

No, I just mean, allow file modules (.tgz) and reject all others. That’s it.

1 Like

So you want one to sign up to a third party, github, to report a security issue?
Umm. Nope. 99% of all software around uses a security@ email address, that ususally goes to an internal mailing list or email forwarders to all coders, that is what you should be doing, rather than trying to throw a hissy towards someone who did allegedly not report it to you, maybe they did, but got nowhere since it didnt meet the hoops and bounds you want them to go through.

and I’ve always scoffed at this 60 bd’s, what you’re actually saying is it might as well be 90 real days, 3 months to fix security issues?

Thats when you accept them, but we are not really surprised, just look at the two biggies, prior to fpbx 16 chanspy was enabled by default, even now being enabled, there is no passwording on it, with sangomas response - ohh but you can set a pin using commercial module XYZ (no you can actually make a simple edit to one file that fixes it, forget where but theres a blog post or 20 out there on how to), then lets not forget 2FA, ohh but that too is a commercial module.

TLDR:
1 - Sangoma make it hard for people to report security issues.
2 - The only way sangoma cares about security is if you pay them to.

3 Likes

2 posts were split to a new topic: Double-check if ChanSpy is enabled

A wild left field knee-jerk opinion that shouldn’t have ever been posted from the “Open Source Solutions Advocate”. Have you done ANY research on the history of this project?

Please take a step back and look at why so many people (even non clearly ip folk) are so heated at what you say.

Take a LONG look. LISTEN to your (few) developers

and note that you said your “delete” comment after Bill posted his advice.

4 Likes