From 3c63cedb5d3defd831eb2d8bfdeb883121737ef3 Mon Sep 17 00:00:00 2001
From: Mark Shroyer <code@markshroyer.com>
Date: Thu, 26 Sep 2013 02:01:21 -0400
Subject: [PATCH] Fix undefined behavior validating XSD substitution groups

A bug in XSD substitution group validation would result in an invalid
cast of SchemaType::Ptr to XsdComplexType::Ptr, in which case evaluating
complexType->prohibitedSubstitutions() exhibited undefined behavior.  In
practice this caused validation against XSD schemas containing
substitution groups to fail on some machines, where ORing the checkSet
mask against out of bounds memory could cause the function

XsdSchemaHelper::substitutionGroupOkTransitive()

to return a false negative.

Minus the bug fix, the regression test added in this commit failed on
(at least) Linux ARM when compiled with the Linaro toolchain 2013.01 g++
4.7, with flags -marm -mcpu=cortex-a8 -O2.  However, it did not fail on
a Linux amd64 machine prior to the bug fix.

Change-Id: Idd060a941a3bc0620f1fcc903375e43022bdcbdc
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
---
 src/xmlpatterns/schema/qxsdschemahelper.cpp       |  2 +-
 .../files/substitution-group-invalid.xml          |  3 +++
 .../files/substitution-group-valid.xml            |  3 +++
 .../files/substitution-group.xsd                  | 15 +++++++++++++++
 .../tst_xmlpatternsvalidator.cpp                  | 12 ++++++++++++
 5 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 tests/auto/xmlpatternsvalidator/files/substitution-group-invalid.xml
 create mode 100644 tests/auto/xmlpatternsvalidator/files/substitution-group-valid.xml
 create mode 100644 tests/auto/xmlpatternsvalidator/files/substitution-group.xsd

diff --git a/src/xmlpatterns/schema/qxsdschemahelper.cpp b/src/xmlpatterns/schema/qxsdschemahelper.cpp
index 8072d926..c5e0ee08 100644
--- a/src/xmlpatterns/schema/qxsdschemahelper.cpp
+++ b/src/xmlpatterns/schema/qxsdschemahelper.cpp
@@ -635,7 +635,7 @@ bool XsdSchemaHelper::substitutionGroupOkTransitive(const XsdElement::Ptr &head,
 
         NamedSchemaComponent::BlockingConstraints checkSet(blockSet);
         checkSet |= head->disallowedSubstitutions();
-        if (head->type()->isComplexType()) {
+        if (head->type()->isComplexType() && head->type()->isDefinedBySchema()) {
             const XsdComplexType::Ptr complexType(head->type());
             checkSet |= complexType->prohibitedSubstitutions();
         }
diff --git a/tests/auto/xmlpatternsvalidator/files/substitution-group-invalid.xml b/tests/auto/xmlpatternsvalidator/files/substitution-group-invalid.xml
new file mode 100644
index 00000000..a4ca0c62
--- /dev/null
+++ b/tests/auto/xmlpatternsvalidator/files/substitution-group-invalid.xml
@@ -0,0 +1,3 @@
+<outer>
+    <innerInvalid>foo</innerInvalid>
+</outer>
diff --git a/tests/auto/xmlpatternsvalidator/files/substitution-group-valid.xml b/tests/auto/xmlpatternsvalidator/files/substitution-group-valid.xml
new file mode 100644
index 00000000..54d81a08
--- /dev/null
+++ b/tests/auto/xmlpatternsvalidator/files/substitution-group-valid.xml
@@ -0,0 +1,3 @@
+<outer>
+    <innerValid>foo</innerValid>
+</outer>
diff --git a/tests/auto/xmlpatternsvalidator/files/substitution-group.xsd b/tests/auto/xmlpatternsvalidator/files/substitution-group.xsd
new file mode 100644
index 00000000..7143bb50
--- /dev/null
+++ b/tests/auto/xmlpatternsvalidator/files/substitution-group.xsd
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified">
+    <xs:element name="outer">
+        <xs:complexType>
+            <xs:choice>
+                <xs:element ref="MySub"/>
+                <xs:element ref="MyBadSub"/>
+            </xs:choice>
+        </xs:complexType>
+    </xs:element>
+    <xs:element name="MySub" abstract="true"/>
+    <xs:element name="MyBadSub" type="xs:string" block="substitution"/>
+    <xs:element name="innerValid" substitutionGroup="MySub" type="xs:string"/>
+    <xs:element name="innerInvalid" substitutionGroup="MyBadSub" type="xs:string"/>
+</xs:schema>
diff --git a/tests/auto/xmlpatternsvalidator/tst_xmlpatternsvalidator.cpp b/tests/auto/xmlpatternsvalidator/tst_xmlpatternsvalidator.cpp
index fb94e091..19a20ba2 100644
--- a/tests/auto/xmlpatternsvalidator/tst_xmlpatternsvalidator.cpp
+++ b/tests/auto/xmlpatternsvalidator/tst_xmlpatternsvalidator.cpp
@@ -228,6 +228,18 @@ void tst_XmlPatternsValidator::xsdSupport_data() const
         << (QStringList() << path + QLatin1String("dateTime-with-microseconds.xml")
                           << path + QLatin1String("dateTime-with-microseconds.xsd"))
         << QString();
+
+    QTest::newRow("A document with a valid substitution group")
+        << 0
+        << (QStringList() << path + QLatin1String("substitution-group-valid.xml")
+                          << path + QLatin1String("substitution-group.xsd"))
+        << QString();
+
+    QTest::newRow("A document attempting to use a prohibited substitution")
+        << 1
+        << (QStringList() << path + QLatin1String("substitution-group-invalid.xml")
+                          << path + QLatin1String("substitution-group.xsd"))
+        << QString();
 }
 
 QTEST_MAIN(tst_XmlPatternsValidator)
-- 
GitLab