Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003267Doomseeker[All Projects] Bugpublic2017-09-19 08:082017-12-13 18:37
ReporterWubTheCaptain 
Assigned ToZalewa 
PriorityurgentSeverityexploitReproducibilityhave not tried
StatusassignedResolutionopen 
PlatformOSMicrosoft Windows & Apple OS XOS Version
Product Version1.1 
Target VersionFixed in Version 
Summary0003267: Auto-updates issued over insecure HTTP, risk of arbitrary code execution
DescriptionAutoUpdater::UPDATER_INFO_URL_BASE (src/core/updater/autoupdater.cpp) is set to serve Doomseeker executable updates over HTTP with no cryptographic signature verification. update-info.js files from the remote directory also contain HTTP URI references to binary downloads.

Doomseeker checks the beforementioned URL on Windows & OS X operating systems and automatically downloads an update with or without confirmation, depending on user preferences.

These should be changed to HTTPS URIs, or a form of cryptographic hash/signature verification to be implemented.
Steps To ReproduceOn the high level: Enable auto-updates and setup a MITM proxy for HTTP requests. Hijack connections tohttp://doomseeker.drdteam.org/updates/ [^] and serve a malicious respective update-info.js file to the user with a different package version. The ZIP file mentioned in update-info.js would carry malicious payload, placed by the bad actor.
Additional InformationThis issue should be made public once a patched build has been released. As a workaround, users should be advised disable automatic updates and fall back to update confirmations for time being.

A CVE-identifier should be assigned, if confirmed.
Attached Files

- Relationships

-  Notes
User avatar (0018338)
WubTheCaptain (developer)
2017-09-19 08:08
edited on: 2017-09-19 08:10

Patch by Zalewa:https://bitbucket.org/Doomseeker/doomseeker/commits/6e4d1b8a6b7339a353619b3f740e972f57187068 [^]

update-info.js files need a change too.

User avatar (0018339)
WubTheCaptain (developer)
2017-09-19 08:15

Can anyone tell if Doomseeker 1.0 is still supported or vulnerable?
User avatar (0018340)
WubTheCaptain (developer)
2017-09-19 08:17

I also have a question, is the X.509 certificate over HTTPS verified to be valid?
User avatar (0018343)
Zalewa (developer)
2017-09-19 10:24

Quote from "WubTheCaptain"
update-info.js files need a change too.

I have already updated them on the server.
User avatar (0018405)
WubTheCaptain (developer)
2017-09-27 22:45

Could we release a security patch for Doomseeker? Old versions still attempt to browse the directory for JSON update files over HTTP, which poses the same risks for MITM.

Every day that passes, bad guys may learn about exploiting this while no public announcement exists for the general public to be aware of.
User avatar (0018410)
Blzut3 (administrator)
2017-10-01 20:04

Zalewa, I'll leave the decision of if we should put out a patch release to you. Personally I don't think this attack is very practical to warrant a hot fix considering it wasn't all that long ago that serving updates over HTTPS would have been impractical for us. Many other small programs were in the same boat and it's not like you hear about this kind of attack actually happening.

On the other hand I don't really want to look like we don't take security seriously, so there is that merit to hot fixing.
User avatar (0018411)
Zalewa (developer)
2017-10-01 22:29

One thing to consider here is that the benefit of attacking a handful of Doom players would be minimal in comparison to the required effort, given that a malicious host would probably need to come from the ISPs for many users. I could imagine a public WiFi access point injecting all downloaded executables with viruses, but in such case any insecure site can be used as an attack vector. However, given how everyone switches to HTTPS nowadays, there wouldn't be many opportunities to utilize such attack. Moreover, any sane Windows user will have an anti-virus installed, which will hopefully curb known malicious patterns. On the other hand, it's not unheard of for exploits to go undetected but be in use to attack people for years, and code is just code - anti-viruses won't detect everything. Ransomware, for example, can go undetected because it's just a piece of software that lists files, reads them and writes them back to disk (in a "slightly" different form.)

I don't think we should trivialize this, though, even if it can be reasonably suspected that no one would take the required effort for the benefit of potentially infecting a few hosts. Since we have also done some other work lately, release 1.1.1 or 1.2 might be in order soon. There's no other way to tell Doomseeker to download the update-info.json from a HTTPS rather than HTTP, and also Doomseeker currently doesn't care if the URLs inside that file are secure or not.

I'm not sure, however, how to protect the users from the case where someone hacks Doomseeker's website and replaces the files there. If they can do that, they can also replace any checksums that we can place there. One way to minimize the risk would be to put checksums on a separate host, with as much different configuration as possible. Mendeley also seems to already do checksum verification, unless that 'hash' field in that XML is just for show, so if we can ensure the security of those XML files then we should get that functionality for free.

Another thing completely are Zandronum testing releases. Zandronum hosting can also get hacked and the game binaries can be replaced, and Doomseeker also installs and then executes those without a second thought.
User avatar (0018412)
Blzut3 (administrator)
2017-10-01 22:59

My plan was to propose a 1.2 release after most of WubTheCaptain's tickets were resolved. Although I don't know how many weeks that will take.

The only sane option for making the updates safer would be to add PCKS7
or GPG signatures and embedding the public key into the updater (similar to the request in ticket 0003275). We don't have the resources to really host hashes on a different server and honestly I don't really think that would actually add any more security.
User avatar (0018415)
WubTheCaptain (developer)
2017-10-02 22:23
edited on: 2017-10-02 22:33

Do Windows and macOS binaries of Doomseeker and Zandronum use code signing, or does this step of verification ever happen at all? What I mean by this is at least in Windows Vista and later, you may get an "unknown publisher" warning or error on execution depending on User Account Control (UAC) setting when they are not signed and make changes to the system. If this happens, it may be a first layer of security.

(My guess is this may not happen, because the releases are portable with no shared installer wizard for portable/non-portable installs. Some wizards can be automated silently too. No idea about macOS, probably much easier to install stuff from the App Store or Homebrew on that operating system.)

Quote from Zalewa
Doomseeker currently doesn't care if the URLs inside that file are secure or not.


It may not have to.

If you make a request to a HTTPS site, without HTTP redirect in between or have HSTS preloaded, and verify the X.509 certificate from response, the files or hashes are good enough to trust to have originated from that server. (This relies on security of DNS and trust of X.509 CAs that we build upon our HTTPS sites, admitted.)

To prevent the situation of replaced files through compromised server, OpenPGP signatures would sign the hashes (0003275). This also works over insecure HTTP/FTP, if the public key is known/trusted (web of trust); You get enough people to confirm "this public key/signature came from a Doomseeker developer" in person face-to-face (at key signing parties).

If Doomseeker used code signed installer wizards to perform updates, Doomseeker's built-in code may not have to care at all where the files came from. The updates would then be verified at the operating system level. (I don't have an answer what would prevent a malicious author from replacing the code signatures in those installers with something trusted, as I'm not familiar with this.)

Zandronum (and Freedoom WADs to an extent) rely more on trust of HTTPS for now. We don't have much control over those, unless upstreams make a change. Old versions use HTTP to discover them, again.

1.2 release may be a bit far off at current pace, so 1.1.1 seems more time-relevant to backport at least until a better solution is in place.

User avatar (0018416)
WubTheCaptain (developer)
2017-10-02 22:30
edited on: 2017-10-02 23:33

To add to the installer wizard proposal (if replacing Mendeley or distributed binaries is desired): WiX, Inno Setup and NSIS are popular libre wizards.

(Windows Installers (MSI) are complex but the official Microsoft supported installation mechanism.)

User avatar (0018434)
Zalewa (developer)
2017-10-04 16:27

Auto-updates need to remain as headless as possible. They need to be smooth, quick, transparent to the user and not install anything that wasn't installed already. I took example from Firefox.

I don't think we should replace Mendeley.
User avatar (0018459)
Zalewa (developer)
2017-10-07 10:09
edited on: 2017-10-07 10:09

By going full-HTTPS we have inadvertedly broken Windows XP support - or at least, an unupdated Windows XP SP3 support. I keep such VM to verify if Doomseeker runs and works properly on Win XP. Because I don't update this machine whatsoever, it has outdated CA root certs. Due to this, some of the HTTPS sites that we try to contact fail to respond. One of those sites washttps://doomseeker.drdteam.org [^] although I cannot reproduce that particular connection failure anymore.

Solutions:

1. Not care. People should not use systems that are past their official support date.

2. Dump CA certs from a contemporary OS into a .pem file, redistribute it with Doomseeker and instruct Qt to load certs from this file.


I explored 2. a bit and conclusions are as follows:

1. Qt code needed to dump cacerts to a .pem file is trivial. Just foreach on the QSslConfiguration::systemCaCertificates() and write them all to one cacerts.pem file usinghttp://doc.qt.io/qt-5/qsslcertificate.html#toPem [^] .

2. The resulting cacerts.pem file is only 50kB, and this is bigger than it should be because I have certs from Kaspersky AV. A "clean" OS will produce an even smaller file. We can put this file into the repository and distribute in doomseeker-core auto-update package.

3. The code to load additional CA certs from this file is also trivial:

QFile certsFile("cacerts.pem");
if (certsFile.exists())
{
    certsFile.open(QIODevice::ReadOnly);
    QSslConfiguration sslConf = QSslConfiguration::defaultConfiguration();
    QList<QSslCertificate> cacerts = sslConf.caCertificates();
    cacerts.append(QSslCertificate::fromDevice(&certsFile));
    sslConf.setCaCertificates(cacerts);
    QSslConfiguration::setDefaultConfiguration(sslConf);
    certsFile.close();
}


Comments/suggestions?

User avatar (0018460)
WubTheCaptain (developer)
2017-10-07 10:30

Drop support, I say. Maintaining root certificates is a great burden and userspace applications should not do that (especially on GNU/Linux, nevermind how autoupdates don't happen there).

I mean, you can still access it over HTTP I suppose:http://doomseeker.drdteam.org/ [^] – there's no HSTS or 301 redirects forcing you to use HTTPS. It's just you won't get automatic updates anymore on Windows XP.

Ideally I would even go full HTTPS some day. I can't support this idea of loading a PEM file even conditionally on Windows XP only. Sorry.
User avatar (0018461)
WubTheCaptain (developer)
2017-10-07 10:38
edited on: 2017-10-07 10:41

Oh, I almost forgot the WAD archives were also switched to HTTPS by default. So were Zandronum releases and Freedoom game data downloads.

...tough shit. It's comparable with asking to weaken security for everyone (every secure system) with 3DES or RC4 ciphersuites only to support Windows XP with TLS. No.

But thank you for taking the time to investigate, anyway! It was interesting to read.

User avatar (0018478)
Blzut3 (administrator)
2017-10-07 23:16

Given that pretty much everyone in this community will be using Lets Encrypt, it'd probably be enough to add their CA cert.
User avatar (0018517)
WubTheCaptain (developer)
2017-10-10 09:49
edited on: 2017-10-10 09:50

Can I get a timeline update on this issue, please? I'd like this issue to be disclosed and communicated to the public by the next two weeks if possible. If it needs more time, please tell.

User avatar (0018532)
Zalewa (developer)
2017-10-11 17:29

I plan on dealing with this properly once we resolve the ticket on licensing.
User avatar (0018534)
WubTheCaptain (developer)
2017-10-11 18:34
edited on: 2017-10-11 18:35

I acknowledge the need to resolve 0003237 before publishing any new releases on Doomseeker's website, and it's reasonable. But could we communicate this to the public with a patch/diff any sooner, please? More than a commit existing in the repository unknown to the public.

User avatar (0018857)
Blzut3 (administrator)
2017-11-11 20:14

As far as I'm concerned you can disclose if you want. This issue is incredibly difficult to exploit with any reasonable expectation of success without specifically targeting someone.
User avatar (0018864)
WubTheCaptain (developer)
2017-11-12 17:14

Issue made public.
User avatar (0018957)
Zalewa (developer)
2017-12-12 14:38

This commit enabled loading of cacert.pem file:https://bitbucket.org/Doomseeker/doomseeker/commits/bffbf95cd7b38fbfa03bc763ef27dbd581cb99ad [^]

This commit actually adds the cacert.pem file to Windows release:https://bitbucket.org/Doomseeker/doomseeker/commits/83cff7c9f1ace55672b2da50a50009e8bc7d6a48 [^]

cacerts.pem contains CA for Let's Encrypt and the one used by github.io so that Freedoom can be installed on Win XP.
User avatar (0018959)
Zalewa (developer)
2017-12-12 17:25

The move to HTTPS is complete (unless I missed something), and despite Wub's advice I also included the CA cert database for github.io's cert and for "Let's Encrypt", as Blzut3 suggested.

Now, we can take it up a notch and PGP the packages.

Qt doesn't come with PGP features, but QCA tool exists (https://userbase.kde.org/QCA [^] ) which seems to do the job. Its license is LGPLv2.1+, so it's compatible with ours. We could even try and figure out how to compile its code into Doomseeker statically. Naturally, QCA code would be added to Doomseeker's repo like Mendeley Updater was. One PITA is that it apparently needs 'gpg' binary to run, which on Linux is granted and on Windows it's the opposite (and also Linux is the only platform where we don't need it).

Embedding gnupg library directly is impossible due to conflicting licenses (GPLv3).

This, of course, means more work, more maintenance, bigger Doomseeker package size and all the other imaginable and unimaginable overheads.

If we were to do this (though I find it unlikely given the problematic implementation), let me know if I get the procedure right:

1. We (ie. The Doomseeker Team) create our own PGP key pair and embed the public key in Doomseeker executable.
2. The update archives and other software packages are created as usual.
3. We calculate hashes (SHA256) from the archives and encrypt those hashes using our private key.
4a. Update packages are made available for download from our server.
4b. PGP'd hashes are also made available for download and sit next to the update packages.
5. Doomseeker downloads the update package and its PGP'd hash.
6a. Doomseeker computes the hash of the downloaded update package.
6b. Doomseeker decrypts the PGP'd hash.
7. Doomseeker does computedHash == decryptedHash.
8. If match - install, if no match - scream in terror.
User avatar (0018961)
WubTheCaptain (developer)
2017-12-13 00:51

I believe cacert.pem is not a creative work in the sense of copyright (at least in the United States), and therefore believe it to be entirely of information that is common property and in public domain.

Doomseeker code doesn't need to know about OpenPGP and all that, if the installer executable is code signed. See 0003292. As long as Mendeley can fetch an MSI file over HTTPS and execute it with silent upgrade option, all good? MSI installers refuse to install on Windows 10 without code signing.

Unrelated: OpenPGP signatures for source tarball releases would be optional, but preferred.
User avatar (0018962)
Blzut3 (administrator)
2017-12-13 04:38

The path of least resistance on signing the update packages would probably to use x509 since the libraries for it are already there for HTTPS. You should be able to use OpenSSL to verify PKCS#7/CMS signatures.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2017-09-19 08:08 WubTheCaptain New Issue
2017-09-19 08:08 WubTheCaptain Note Added: 0018338
2017-09-19 08:10 WubTheCaptain Note Edited: 0018338 View Revisions
2017-09-19 08:15 WubTheCaptain Note Added: 0018339
2017-09-19 08:17 WubTheCaptain Note Added: 0018340
2017-09-19 10:24 Zalewa Note Added: 0018343
2017-09-27 22:45 WubTheCaptain Note Added: 0018405
2017-10-01 20:04 Blzut3 Note Added: 0018410
2017-10-01 22:29 Zalewa Note Added: 0018411
2017-10-01 22:59 Blzut3 Note Added: 0018412
2017-10-02 22:23 WubTheCaptain Note Added: 0018415
2017-10-02 22:30 WubTheCaptain Note Added: 0018416
2017-10-02 22:33 WubTheCaptain Note Edited: 0018415 View Revisions
2017-10-02 23:23 WubTheCaptain Note Edited: 0018416 View Revisions
2017-10-02 23:33 WubTheCaptain Note Edited: 0018416 View Revisions
2017-10-04 16:27 Zalewa Note Added: 0018434
2017-10-05 02:49 WubTheCaptain Status new => acknowledged
2017-10-07 10:09 Zalewa Note Added: 0018459
2017-10-07 10:09 Zalewa Note Edited: 0018459 View Revisions
2017-10-07 10:30 WubTheCaptain Note Added: 0018460
2017-10-07 10:38 WubTheCaptain Note Added: 0018461
2017-10-07 10:41 WubTheCaptain Note Edited: 0018461 View Revisions
2017-10-07 23:16 Blzut3 Note Added: 0018478
2017-10-10 09:49 WubTheCaptain Note Added: 0018517
2017-10-10 09:50 WubTheCaptain Note Edited: 0018517 View Revisions
2017-10-10 09:55 WubTheCaptain Summary Auto-updates issued over insecure HTTP, risk of arbitrary arbitrary code execution => Auto-updates issued over insecure HTTP, risk of arbitrary code execution
2017-10-11 17:29 Zalewa Note Added: 0018532
2017-10-11 18:34 WubTheCaptain Note Added: 0018534
2017-10-11 18:35 WubTheCaptain Note Edited: 0018534 View Revisions
2017-11-11 11:26 Zalewa Assigned To => Zalewa
2017-11-11 11:26 Zalewa Status acknowledged => assigned
2017-11-11 20:14 Blzut3 Note Added: 0018857
2017-11-12 17:14 WubTheCaptain Note Added: 0018864
2017-11-12 17:14 WubTheCaptain View Status private => public
2017-12-12 14:38 Zalewa Note Added: 0018957
2017-12-12 17:25 Zalewa Note Added: 0018959
2017-12-12 17:25 Zalewa Status assigned => feedback
2017-12-13 00:51 WubTheCaptain Note Added: 0018961
2017-12-13 00:51 WubTheCaptain Status feedback => assigned
2017-12-13 04:38 Blzut3 Note Added: 0018962
2017-12-13 18:37 Filystea Note Added: 0018963
2017-12-14 00:48 Blzut3 Note Deleted: 0018963






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker