Opened 8 years ago

Last modified 14 months ago

#4074 new defect

Mongoose outdated

Reported by: susnux Owned by:
Priority: Should Have Milestone: Backlog
Component: Build & Packages Keywords:
Cc: Patch:

Description (last modified by Langbart)

When compiling 0AD (alpha 20) for openSUSE I get following warning about a potential security issue:

W: missing-call-to-setgroups-before-setuid /usr/bin/pyrogenesis
This executable is calling setuid and setgid without setgroups or initgroups.
There is a high probability this means it didn't relinquish all groups, and
this would be a potential security issue to be fixed. Seek POS36-C on the web
for details about the problem.

And also I get this warning:

I: binary-or-shlib-calls-gethostbyname /usr/bin/pyrogenesis
The binary calls gethostbyname(). Please port the code to use getaddrinfo().

gethostbyname should not be used because it does not support IPv6 and in the near future there might be some IPv6 only networks (e.g. at the moment my provider only gives me an IPv6 range and IPv4 only as carrier-grade-nat).

Change History (12)

comment:1 by fsincos, 7 years ago

Description: modified (diff)

Disclaimer: I'm definitely not a security expert (I'm actually a mathematician, my programming "competence" is mostly "by proxy"), so what I write here might not make sense.

I've searched the source code and found "setuid" in the following places:

  • ./binaries/data/mods/public/simulation/ai/common-api/shared.js and entitycollection.js.

These seem to not be the real deal*.

  • gloox, again, probably not the real thing*.
  • mongoose. We use a version from 2011(!), since then the development seems to have moved from Hg to git (https://github.com/cesanta/mongoose seems to be where it's now) and, obviously, the source code is completely different. It says it's an embedded web server library; I know very little about these things so I can't help with updating it.
  • These seem to be custom functions that set a variable called _uid or m_uid.

The situation for "gethostbyname" is as follows:

  • mongoose is at it again.

So, to conclude, the culprit is likely that ancient version of mongoose (and who can blame it, back then people were still typing on stone tablets). It would be an "easy" fix to update it, but since this kind of thing is going to be (and already is for some other libraries we use) a recurring theme, we should probably also create a wiki page (or something like it) where third party libraries are tracked together with their upstream locations, related bugs/issues (e.g. this) and our porting/updating/staging/... branches. Or, if that already exists, it should be way more front-and-center (I couldn't find it on the Wiki at a quick glance).

I'd like to apologize. It's certainly not ideal that security concerns/bugs (however exploitable they might be) stand unanswered for this long. It's also unfortunate that little effort went into keeping up with (some) third-party library updates. There is no point in laying blame somewhere, I just hope we can make sure these problems get fixed before they blow up in our faces.

PS: I'm slightly pissed that I had to step way out of my area of competence here because some people think it's cool to ignore a distribution's maintainer for our package or something. It's possible it was decided to keep the fix private/secret until it was ready; anyway, you could have at least written something like "acknowledged" or a more informed version of what I cobbled together.

comment:2 by Stan, 7 years ago

I hadn't seen that ticket at all. What I can say from what I read as it is also out of my reach is that us using (gethostbyname();) doesn't matter much right now because enet.h currently doesn't have IPV6 support. We'd have to merge a modified version from another repo.

Thanks for answering fsincos.

in reply to:  1 comment:3 by elexis, 7 years ago

Component: UI & SimulationBuild & Packages
Priority: Must HaveShould Have
Summary: Potential security issue and warnings when compilingMoongoose outdated

The setUID of the AI javascript code has nothing to do with the compiler warning about setuid, which is indeed only used in mongoose.

The mongoose webserver is only used by the profiler, so the webserver is not exposed publicly but only to the localhost (maybe there is some weird way to achieve something malicious, but appears unlikely)

As Stan mentioned, the enet library used by 0ad has no ipv6 support at all (there is a separate ticket for this). However since gethostbyname is only used by moongoose, that issue is entirely irrelevant, as that only refers to the localhost.

Replying to fsincos:

PS: I'm slightly pissed that I had to step way out of my area of competence here

Agree, though broadening ones own capabilities is inevitable with a project of this size where most original developers have left long ago, in particular the ones adding the code that left presents for us: http://trac.wildfiregames.com/log/ps/trunk/source/third_party/mongoose

in reply to:  description comment:4 by leper, 7 years ago

Keywords: Security removed

Replying to susnux:

When compiling 0AD (alpha 20) for openSUSE I get following warning about a potential security issue:

W: missing-call-to-setgroups-before-setuid /usr/bin/pyrogenesis
This executable is calling setuid and setgid without setgroups or initgroups.
There is a high probability this means it didn't relinquish all groups, and
this would be a potential security issue to be fixed. Seek POS36-C on the web
for details about the problem.

Given the research done by others, this is not an issue. Given that mongoose is only started for the profiler, and only listens on localhost (and if you don't trust your local machine, or forward those ports...). Also we don't pass the options for either of those two functions, so the embedded webserver (that is used only for the profiler) runs with the same priviledges as the normal user.

Updating mongoose could be an option if we rip out the lobby. Mongoose changed to GPL2 (not "or later") sometime in 2012, gloox changed to GPL3 (also no "or later") with 1.0.9 IIRC. So we could update that, but we could not distribute those binaries.

That said if someone finds a nicely licensed (I'd prefer something liberally licensed, but GPL2+ would also work) web server (or xmpp library) that works and isn't huge (mongoose uses two files (one of which is a header)).

comment:5 by elexis, 7 years ago

Summary: Moongoose outdatedMongoose outdated

comment:6 by Langbart, 14 months ago

Description: modified (diff)
  • similar ticket closed #6751

comment:7 by Stan, 14 months ago

I've done a bit more digging and it seems the switch from MIT to GPL 2.0 was done with blatant disregard of copyright and of contributors. I'm not sure this is something we want to encourage.

Given this information the latest version we can use is 3.9 (Latest is 7.9)

There is

Maybe https://gitlab.com/eidheim/Simple-Web-Server depends on Boost::asio might be an issue.

Mongoose seemed to be the only two files one :(

comment:8 by phosit, 14 months ago

I doubt that we need a http-server inside 0ad.

  • Profiler2: I don't need to comunicate to a browser if I could save a report to a file(and opon that in the browser).
  • RL-Interface: Surely there is a more suitable IPC protocol for that.

in reply to:  7 comment:9 by Ralph Sennhauser, 14 months ago

Replying to Stan`:

I've done a bit more digging and it seems the switch from MIT to GPL 2.0 was done with blatant disregard of copyright and of contributors. I'm not sure this is something we want to encourage.

Given this information the latest version we can use is 3.9 (Latest is 7.9)

That latest you linked is still using pthread, so blocks removal of pthread wrapper in lib

There is

Maybe https://gitlab.com/eidheim/Simple-Web-Server depends on Boost::asio might be an issue.

Crow uses standalone asio ;)

comment:10 by Stan, 14 months ago

Profiler2: I don't need to comunicate to a browser if I could save a report to a file(and opon that in the browser).

You can already do that. But you can also connect live to the game, to debug something at a given moment which is useful IMHO.

RL-Interface: Surely there is a more suitable IPC protocol for that.

Sure Initially we wanted to use GRPC, but it's pain to compile on Windows.

That latest you linked is still using pthread, so blocks removal of pthread wrapper in lib

Well that's least worrysome than license issues, to me at leasT.

HTTP also is really easy to use. I can use postman for it. Scripts can call the game easily.

Crow uses standalone asio ;)

Right.

in reply to:  10 comment:11 by Ralph Sennhauser, 14 months ago

Replying to Stan`:

That latest you linked is still using pthread, so blocks removal of pthread wrapper in lib

Well that's least worrysome than license issues, to me at leasT.

The gpl2-only makes even later versions unusable due to incompatibility with gpl3, but I see little reason to update to a version that doesn't fix the pthread dependency, meaning the current version is as good as that latest MIT version.

comment:12 by Stan, 14 months ago

Well it's more about having up to date libs :) And since gloox is GPL3 we can't use later versions.

Note: See TracTickets for help on using tickets.