New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XXE in PwDatabaseV4 #200

Open
ProDigySML opened this Issue Oct 24, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@ProDigySML
Copy link

ProDigySML commented Oct 24, 2018

The Issue

An XML External Entity attack is a type of attack against an application that parses XML input. This attack occurs when XML input containing a reference to an external entity is processed by a weakly configured XML parser. This attack may lead to the disclosure of confidential data, denial of service, server side request forgery, port scanning from the perspective of the machine where the parser is located, and other system impacts.

Where the Issue Occurred:

DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
DocumentBuilder db = dbf.newDocumentBuilder();
Document doc = db.parse(keyInputStream);

Remediation

https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet

@J-Jamet

This comment has been minimized.

Copy link
Member

J-Jamet commented Oct 25, 2018

Thank you for this security reply. I will look at the solutions to make a patch. Do you have an example of a kdbx file with a link that will defeat the XML parser?

@J-Jamet J-Jamet added the enhancement label Oct 25, 2018

@nightbritenitelite

This comment has been minimized.

Copy link

nightbritenitelite commented Nov 8, 2018

Throwaway account, but wanted to put some info out there.

Overview

KeePassDX (as of now) does not have the required permissions for internet access (not sure about localhost - that is potentially another attack vector if KeePassDX does open itself up in any way over localhost of course).

This doesn't mean that KeePassDX isn't vulnerable to XXE attacks (it is), but when it comes to stealing passwords with this attack via sending payloads to an external server, the way in which an attack like this is conducted is severely limited.

On the other hand, MANY other attacks that utilize XXE-processing exploits are still possible, and aren't affected by the lack of internet access!

As a Side Note:
I don't have the time right now to create a test KDBX file, but I do know that with the way KeePassDX is handling XML via the DBF (Document Builder Factory), it is in-fact vulnerable to XXE (XML External Entity) processing attacks.

Hypothetical DOS Attack Example

One such attack would be a Denial of Service attack where the app would crash upon attempting to parse a KDBX file. The attack would assume that:

  • The attacker has exploited a security whole within the attack vector, which not only includes KeePassDX but everything involving the KDBX file's device storage, device-to-device transferring, all types of access, etc.)
  • Using this security whole, the attacker modifies the KDBX file to include something like the following:
...
<!DOCTYPE foo [
<!ELEMENT foo ANY >
<!ENTITY xxe SYSTEM "file:///dev/block/loop0">]><foo>&xxe;</foo>
...

By referencing a device (/dev/*) that is inaccessible to non-root apps, a security exception will be thrown in KeePassDX, causing the app to crash (if unhandled).

This is just one of many potential attacks and attack vectors that XXE processing allows for.

Patching the Exploit:

The Android Developer documentation for the XMLConstants class only exposes one feature that we can turn off to prevent this attack (FEATURE_SECURE_PROCESSING). However, let it be noted that I have not tested to see if this feature was implemented correctly in the Android SDK. It is possible that the DocumentBuilderFactory's XML parser is not implemented correctly, and that it will still be vulnerable to an XXE-processing exploit, but while possible, it's improbable for this to be the case as it has been implemented since SDK Level 1.

Saying this, I highly suggest that this is checked (or further researched to see if this has been checked) before fully accepting the following solution. I also suggest researching more about this before you implement a fix for it and accept the fix as a fully-secure solution.

Nevertheless, here is the solution to be implemented for ALL database versions and their respective classes. The code may need to be refactored and an additional class added to handle this without needing to think about it when supporting future database versions, but this decision is left up to the community.

The Solution:
You should use DocumentBuilderFactory's setFeature(...) to disable XXE processing features like such:

DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();

// Disable certain unsecure XML-Parsing DocumentBuilderFactory features
try {
    dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (ParserConfigurationException) { } // Do something!

// The rest is used like normal...
DocumentBuilder db = dbf.newDocumentBuilder(); 
Document doc = db.parse(keyInputStream); 

Websites to look at:
Android Developer's Reference: DocumentBuilderFactory
OWASP XXE General Info
OWASP XXE Prevention Cheat Sheet
"XXE defence(les)s in JDK XML parsers"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment