From 6d215c0b4ebce4957e4541f2cb6cb0bcb154a438 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez?= <jaime.perez@uninett.no>
Date: Thu, 28 Jul 2016 17:14:46 +0200
Subject: [PATCH] Use AttributeValue serializable objects instead of dumping
 manually the XML contents.

This way, we avoid completely any possible XXE attack, and simplify the code as we don't need to deal directly with the DOM. The entire AttributeValue will be saved to the backend as XML, and then recovered back when unserializing.
---
 lib/SimpleSAML/Session.php | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php
index 91d65b2b0..bdc84fd1f 100644
--- a/lib/SimpleSAML/Session.php
+++ b/lib/SimpleSAML/Session.php
@@ -225,11 +225,9 @@ class SimpleSAML_Session implements Serializable
             }
 
             foreach ($parameters['RawAttributes'] as $attribute => $values) {
-                foreach ($values as $idx => $value) {
-                    // this should be originally a DOMNodeList
-                    $dom = new \DOMDocument();
-                    $dom->loadXML($value);
-                    $this->authData[$authority]['Attributes'][$attribute][$idx] = $dom->childNodes;
+                foreach ($values as $idx => $value) { // this should be originally a DOMNodeList
+                    /* @var \SAML2\XML\saml\AttributeValue $value */
+                    $this->authData[$authority]['Attributes'][$attribute][$idx] = $value->element->childNodes;
                 }
             }
         }
@@ -626,17 +624,9 @@ class SimpleSAML_Session implements Serializable
                     continue;
                 }
 
-                // ... and we have at least one DOMElement in there, so we dump back to XML to be able to serialize
-                $original = $value->item(0)->ownerDocument;
-                $new = new DOMDocument($original->version, $original->encoding);
-                $n = $value->length;
-                for ($i = 0; $i < $n; $i++) {
-                    $new->appendChild($new->importNode($value->item($i), true));
-                }
-                $new->saveXML();
-
-                // save the XML representation to 'RawAttributes', using the same attribute name and index
-                $data['RawAttributes'][$attribute][$idx] = $new->saveXML();
+                // create an AttributeValue object and save it to 'RawAttributes', using same attribute name and index
+                $attrval = new \SAML2\XML\saml\AttributeValue($value->item(0)->parentNode);
+                $data['RawAttributes'][$attribute][$idx] = $attrval;
             }
         }
 
-- 
GitLab