MantisBT - Doomseeker
View Issue Details
0003267Doomseeker[All Projects] Securitypublic2017-09-19 08:082019-06-13 13:30
WubTheCaptain 
 
highexploithave not tried
acknowledgedopen 
Microsoft Windows & Apple OS X
1.1 
 
0003267: Auto-updates issued over insecure HTTP, risk of arbitrary code execution
AutoUpdater::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.
On 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.
This 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.
No tags attached.
Issue History
2017-09-19 08:08WubTheCaptainNew Issue
2017-09-19 08:08WubTheCaptainNote Added: 0018338
2017-09-19 08:10WubTheCaptainNote Edited: 0018338bug_revision_view_page.php?bugnote_id=18338#r10970
2017-09-19 08:15WubTheCaptainNote Added: 0018339
2017-09-19 08:17WubTheCaptainNote Added: 0018340
2017-09-19 10:24ZalewaNote Added: 0018343
2017-09-27 22:45WubTheCaptainNote Added: 0018405
2017-10-01 20:04Blzut3Note Added: 0018410
2017-10-01 22:29ZalewaNote Added: 0018411
2017-10-01 22:59Blzut3Note Added: 0018412
2017-10-02 22:23WubTheCaptainNote Added: 0018415
2017-10-02 22:30WubTheCaptainNote Added: 0018416
2017-10-02 22:33WubTheCaptainNote Edited: 0018415bug_revision_view_page.php?bugnote_id=18415#r11020
2017-10-02 23:23WubTheCaptainNote Edited: 0018416bug_revision_view_page.php?bugnote_id=18416#r11022
2017-10-02 23:33WubTheCaptainNote Edited: 0018416bug_revision_view_page.php?bugnote_id=18416#r11023
2017-10-04 16:27ZalewaNote Added: 0018434
2017-10-05 02:49WubTheCaptainStatusnew => acknowledged
2017-10-07 10:09ZalewaNote Added: 0018459
2017-10-07 10:09ZalewaNote Edited: 0018459bug_revision_view_page.php?bugnote_id=18459#r11062
2017-10-07 10:30WubTheCaptainNote Added: 0018460
2017-10-07 10:38WubTheCaptainNote Added: 0018461
2017-10-07 10:41WubTheCaptainNote Edited: 0018461bug_revision_view_page.php?bugnote_id=18461#r11064
2017-10-07 23:16Blzut3Note Added: 0018478
2017-10-10 09:49WubTheCaptainNote Added: 0018517
2017-10-10 09:50WubTheCaptainNote Edited: 0018517bug_revision_view_page.php?bugnote_id=18517#r11124
2017-10-10 09:55WubTheCaptainSummaryAuto-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:29ZalewaNote Added: 0018532
2017-10-11 18:34WubTheCaptainNote Added: 0018534
2017-10-11 18:35WubTheCaptainNote Edited: 0018534bug_revision_view_page.php?bugnote_id=18534#r11126
2017-11-11 11:26ZalewaAssigned To => Zalewa
2017-11-11 11:26ZalewaStatusacknowledged => assigned
2017-11-11 20:14Blzut3Note Added: 0018857
2017-11-12 17:14WubTheCaptainNote Added: 0018864
2017-11-12 17:14WubTheCaptainView Statusprivate => public
2017-12-12 14:38ZalewaNote Added: 0018957
2017-12-12 17:25ZalewaNote Added: 0018959
2017-12-12 17:25ZalewaStatusassigned => feedback
2017-12-13 00:51WubTheCaptainNote Added: 0018961
2017-12-13 00:51WubTheCaptainStatusfeedback => assigned
2017-12-13 04:38Blzut3Note Added: 0018962
2017-12-13 18:37FilysteaNote Added: 0018963
2017-12-14 00:48Blzut3Note Deleted: 0018963
2018-02-14 11:00ZalewaAssigned ToZalewa =>
2018-02-14 11:15ZalewaNote Added: 0019036
2018-02-14 11:53ZalewaStatusassigned => acknowledged
2018-12-04 00:12WubTheCaptainCategoryBug => Security
2018-12-17 05:00WubTheCaptainNote Added: 0020263
2019-06-13 13:30WubTheCaptainPriorityurgent => high

Notes
(0018338)
WubTheCaptain   
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.

(0018339)
WubTheCaptain   
2017-09-19 08:15   
Can anyone tell if Doomseeker 1.0 is still supported or vulnerable?
(0018340)
WubTheCaptain   
2017-09-19 08:17   
I also have a question, is the X.509 certificate over HTTPS verified to be valid?
(0018343)
Zalewa   
2017-09-19 10:24   
Quote from "WubTheCaptain"
update-info.js files need a change too.

I have already updated them on the server.
(0018405)
WubTheCaptain   
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.
(0018410)
Blzut3   
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.
(0018411)
Zalewa   
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.
(0018412)
Blzut3   
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.
(0018415)
WubTheCaptain   
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.

(0018416)
WubTheCaptain   
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.)

(0018434)
Zalewa   
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.
(0018459)
Zalewa   
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?

(0018460)
WubTheCaptain   
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.
(0018461)
WubTheCaptain   
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.

(0018478)
Blzut3   
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.
(0018517)
WubTheCaptain   
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.

(0018532)
Zalewa   
2017-10-11 17:29   
I plan on dealing with this properly once we resolve the ticket on licensing.
(0018534)
WubTheCaptain   
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.

(0018857)
Blzut3   
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.
(0018864)
WubTheCaptain   
2017-11-12 17:14   
Issue made public.
(0018957)
Zalewa   
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.
(0018959)
Zalewa   
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.
(0018961)
WubTheCaptain   
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.
(0018962)
Blzut3   
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.
(0019036)
Zalewa   
2018-02-14 11:15   
I did some additional investigation into using OpenSSL to sign & verify.

For creating the packages I think we can safely assume that "openssl" executable will be somewhere in the path, even on Windows. "COMPILING.Windows.txt" already explains where to get the precompiled package. So, Ruby scripts can try to use OpenSSL to create the .sig files for each .zip. User will know that OpenSSL is missing when .sig files don't get created.

Command lines to do this:


# Spawn private key.
openssl genpkey -algorithm RSA -out private_key.pem -pkeyopt rsa_keygen_bits:2048
# Get public key from private key.
openssl rsa -pubout -in private_key.pem -out public_key.pem
# Create package signature.
openssl dgst -sha256 -sign private_key.pem update_package.zip


Verification is another can of worms. Public key can be embedded as a Qt resource. However, unless I mislooked something, I couldn't find any way to do the verification itself directly from Qt.

Instead, we could LoadLibrary the libeay32.dll and do the verification by using OpenSSL's API directly. Relevant tutorial:https://wiki.openssl.org/index.php/EVP_Signing_and_Verifying [^]

I haven't checked yet if this is feasible.


We will also need to create .sig files for the current stable release or otherwise downgrading from beta to stable won't be possible anymore because auto updater must refuse to work when .sig files are missing.
(0020263)
WubTheCaptain   
2018-12-17 05:00   
Fyi: We have .sig files since Doomseeker 1.2 release now.