Swift
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoanna Hulboj <joanna.hulboj@isode.com>2019-09-12 08:54:19 (GMT)
committerJoanna Hulboj <joanna.hulboj@isode.com>2019-09-16 08:17:07 (GMT)
commit181ac4a83ba4a82be683fb0a6f08393d3c91320c (patch)
tree76e41aac0cda8be5582137d34cb0c9f5683c9dc2
parent415870c04a7e6cabf13e6acf3a94bb0f68732907 (diff)
downloadswift-181ac4a83ba4a82be683fb0a6f08393d3c91320c.zip
swift-181ac4a83ba4a82be683fb0a6f08393d3c91320c.tar.bz2
Close the stream for disallowed XML features
According to RFC 6120 if any disallowed XML feature is encountered, we should close the stream with a <restricted-xml/>. The following features of XML are prohibited in XMPP: - processing instructions - internal or external DTD subsets - internal or external entity references - comments Test-information: Unit tests pass on Windows 10 and Ubuntu 18.04.1 LTS Change-Id: I475920c91b7f9da51ab37c106a4783a52f6e3cae
Diffstat
-rw-r--r--Sluift/ElementConvertors/DOMElementConvertor.cpp2
-rw-r--r--Swift/Controllers/Settings/XMLSettingsProvider.cpp2
-rw-r--r--Swiften/Parser/ExpatParser.cpp18
-rw-r--r--Swiften/Parser/ExpatParser.h2
-rw-r--r--Swiften/Parser/LibXMLParser.cpp30
-rw-r--r--Swiften/Parser/LibXMLParser.h5
-rw-r--r--Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h2
-rw-r--r--Swiften/Parser/PlatformXMLParserFactory.cpp6
-rw-r--r--Swiften/Parser/PlatformXMLParserFactory.h2
-rw-r--r--Swiften/Parser/UnitTest/XMLParserTest.cpp42
-rw-r--r--Swiften/Parser/XMLParser.cpp2
-rw-r--r--Swiften/Parser/XMLParser.h7
-rw-r--r--Swiften/Parser/XMLParserFactory.h2
13 files changed, 105 insertions, 17 deletions
diff --git a/Sluift/ElementConvertors/DOMElementConvertor.cpp b/Sluift/ElementConvertors/DOMElementConvertor.cpp
index 56b45aa..5e72cc8 100644
--- a/Sluift/ElementConvertors/DOMElementConvertor.cpp
+++ b/Sluift/ElementConvertors/DOMElementConvertor.cpp
@@ -185,7 +185,7 @@ boost::optional<std::string> DOMElementConvertor::convertToLua(
185 185
186 // Parse the payload again 186 // Parse the payload again
187 ParserClient parserClient(L); 187 ParserClient parserClient(L);
188 std::shared_ptr<XMLParser> parser(parsers.createXMLParser(&parserClient)); 188 std::shared_ptr<XMLParser> parser(parsers.createXMLParser(&parserClient, false));
189 bool result = parser->parse(serializedPayload); 189 bool result = parser->parse(serializedPayload);
190 assert(result); 190 assert(result);
191 191
diff --git a/Swift/Controllers/Settings/XMLSettingsProvider.cpp b/Swift/Controllers/Settings/XMLSettingsProvider.cpp
index 2573af0..8415209 100644
--- a/Swift/Controllers/Settings/XMLSettingsProvider.cpp
+++ b/Swift/Controllers/Settings/XMLSettingsProvider.cpp
@@ -18,7 +18,7 @@ namespace Swift {
18XMLSettingsProvider::XMLSettingsProvider(const std::string& xmlConfig) : level_(0) { 18XMLSettingsProvider::XMLSettingsProvider(const std::string& xmlConfig) : level_(0) {
19 if (!xmlConfig.empty()) { 19 if (!xmlConfig.empty()) {
20 PlatformXMLParserFactory factory; 20 PlatformXMLParserFactory factory;
21 auto parser = factory.createXMLParser(this); 21 auto parser = factory.createXMLParser(this, true);
22 if (parser->parse(xmlConfig)) { 22 if (parser->parse(xmlConfig)) {
23 SWIFT_LOG(debug) << "Found and parsed system config" << std::endl; 23 SWIFT_LOG(debug) << "Found and parsed system config" << std::endl;
24 } 24 }
diff --git a/Swiften/Parser/ExpatParser.cpp b/Swiften/Parser/ExpatParser.cpp
index a50949b..640d561 100644
--- a/Swiften/Parser/ExpatParser.cpp
+++ b/Swiften/Parser/ExpatParser.cpp
@@ -72,8 +72,21 @@ static void handleEntityDeclaration(void* parser, const XML_Char*, int, const XM
72 static_cast<ExpatParser*>(parser)->stopParser(); 72 static_cast<ExpatParser*>(parser)->stopParser();
73} 73}
74 74
75static void handleComment(void* parser, const XML_Char* /*data*/) {
76 if (!static_cast<ExpatParser*>(parser)->allowsComments()) {
77 static_cast<ExpatParser*>(parser)->stopParser();
78 }
79}
80
81static void handleProcessingInstruction(void* parser, const XML_Char* /*target*/, const XML_Char* /*data*/) {
82 static_cast<ExpatParser*>(parser)->stopParser();
83}
84
85static void handleDoctypeDeclaration(void* parser, const XML_Char* /*doctypeName*/, const XML_Char* /*sysid*/, const XML_Char* /*pubid*/, int /*has_internal_subset*/) {
86 static_cast<ExpatParser*>(parser)->stopParser();
87}
75 88
76ExpatParser::ExpatParser(XMLParserClient* client) : XMLParser(client), p(new Private()) { 89ExpatParser::ExpatParser(XMLParserClient* client, bool allowComments) : XMLParser(client, allowComments), p(new Private()) {
77 p->parser_ = XML_ParserCreateNS("UTF-8", NAMESPACE_SEPARATOR); 90 p->parser_ = XML_ParserCreateNS("UTF-8", NAMESPACE_SEPARATOR);
78 XML_SetUserData(p->parser_, this); 91 XML_SetUserData(p->parser_, this);
79 XML_SetElementHandler(p->parser_, handleStartElement, handleEndElement); 92 XML_SetElementHandler(p->parser_, handleStartElement, handleEndElement);
@@ -81,6 +94,9 @@ ExpatParser::ExpatParser(XMLParserClient* client) : XMLParser(client), p(new Pri
81 XML_SetXmlDeclHandler(p->parser_, handleXMLDeclaration); 94 XML_SetXmlDeclHandler(p->parser_, handleXMLDeclaration);
82 XML_SetEntityDeclHandler(p->parser_, handleEntityDeclaration); 95 XML_SetEntityDeclHandler(p->parser_, handleEntityDeclaration);
83 XML_SetNamespaceDeclHandler(p->parser_, handleNamespaceDeclaration, nullptr); 96 XML_SetNamespaceDeclHandler(p->parser_, handleNamespaceDeclaration, nullptr);
97 XML_SetCommentHandler(p->parser_, handleComment);
98 XML_SetProcessingInstructionHandler(p->parser_, handleProcessingInstruction);
99 XML_SetDoctypeDeclHandler(p->parser_, handleDoctypeDeclaration, nullptr);
84} 100}
85 101
86ExpatParser::~ExpatParser() { 102ExpatParser::~ExpatParser() {
diff --git a/Swiften/Parser/ExpatParser.h b/Swiften/Parser/ExpatParser.h
index 7583339..34d790d 100644
--- a/Swiften/Parser/ExpatParser.h
+++ b/Swiften/Parser/ExpatParser.h
@@ -16,7 +16,7 @@
16namespace Swift { 16namespace Swift {
17 class SWIFTEN_API ExpatParser : public XMLParser, public boost::noncopyable { 17 class SWIFTEN_API ExpatParser : public XMLParser, public boost::noncopyable {
18 public: 18 public:
19 ExpatParser(XMLParserClient* client); 19 ExpatParser(XMLParserClient* client, bool allowComments = false);
20 ~ExpatParser(); 20 ~ExpatParser();
21 21
22 bool parse(const std::string& data, bool finalData = false); 22 bool parse(const std::string& data, bool finalData = false);
diff --git a/Swiften/Parser/LibXMLParser.cpp b/Swiften/Parser/LibXMLParser.cpp
index 192f44b..4e02059 100644
--- a/Swiften/Parser/LibXMLParser.cpp
+++ b/Swiften/Parser/LibXMLParser.cpp
@@ -59,6 +59,24 @@ static void handleCharacterData(void* parser, const xmlChar* data, int len) {
59 static_cast<XMLParser*>(parser)->getClient()->handleCharacterData(std::string(reinterpret_cast<const char*>(data), static_cast<size_t>(len))); 59 static_cast<XMLParser*>(parser)->getClient()->handleCharacterData(std::string(reinterpret_cast<const char*>(data), static_cast<size_t>(len)));
60} 60}
61 61
62static void handleComment(void* parser, const xmlChar* /*data*/) {
63 if (!static_cast<LibXMLParser*>(parser)->allowsComments()) {
64 static_cast<LibXMLParser*>(parser)->stopParser();
65 }
66}
67
68static void handleEntityDeclaration(void * parser, const xmlChar* /*name*/, int /*type*/, const xmlChar* /*publicId*/, const xmlChar* /*systemId*/, xmlChar* /*content*/) {
69 static_cast<LibXMLParser*>(parser)->stopParser();
70}
71
72static void handleProcessingInstruction(void* parser, const xmlChar* /*target*/, const xmlChar* /*data*/) {
73 static_cast<LibXMLParser*>(parser)->stopParser();
74}
75
76static void handleExternalSubset(void* parser, const xmlChar * /*name*/, const xmlChar * /*ExternalID*/, const xmlChar * /*SystemID*/) {
77 static_cast<LibXMLParser*>(parser)->stopParser();
78}
79
62static void handleError(void*, const char* /*m*/, ... ) { 80static void handleError(void*, const char* /*m*/, ... ) {
63 /* 81 /*
64 va_list args; 82 va_list args;
@@ -73,7 +91,7 @@ static void handleWarning(void*, const char*, ... ) {
73 91
74bool LibXMLParser::initialized = false; 92bool LibXMLParser::initialized = false;
75 93
76LibXMLParser::LibXMLParser(XMLParserClient* client) : XMLParser(client), p(new Private()) { 94LibXMLParser::LibXMLParser(XMLParserClient* client, bool allowComments) : XMLParser(client, allowComments), p(new Private()) {
77 // Initialize libXML for multithreaded applications 95 // Initialize libXML for multithreaded applications
78 if (!initialized) { 96 if (!initialized) {
79 xmlInitParser(); 97 xmlInitParser();
@@ -87,6 +105,10 @@ LibXMLParser::LibXMLParser(XMLParserClient* client) : XMLParser(client), p(new P
87 p->handler_.characters = &handleCharacterData; 105 p->handler_.characters = &handleCharacterData;
88 p->handler_.warning = &handleWarning; 106 p->handler_.warning = &handleWarning;
89 p->handler_.error = &handleError; 107 p->handler_.error = &handleError;
108 p->handler_.comment = &handleComment;
109 p->handler_.entityDecl = &handleEntityDeclaration;
110 p->handler_.processingInstruction = &handleProcessingInstruction;
111 p->handler_.externalSubset = &handleExternalSubset;
90 112
91 p->context_ = xmlCreatePushParserCtxt(&p->handler_, this, nullptr, 0, nullptr); 113 p->context_ = xmlCreatePushParserCtxt(&p->handler_, this, nullptr, 0, nullptr);
92 xmlCtxtUseOptions(p->context_, XML_PARSE_NOENT); 114 xmlCtxtUseOptions(p->context_, XML_PARSE_NOENT);
@@ -106,6 +128,7 @@ bool LibXMLParser::parse(const std::string& data, bool finalData) {
106 if (xmlParseChunk(p->context_, data.c_str(), static_cast<int>(data.size()), finalData) == XML_ERR_OK) { 128 if (xmlParseChunk(p->context_, data.c_str(), static_cast<int>(data.size()), finalData) == XML_ERR_OK) {
107 return true; 129 return true;
108 } 130 }
131 if (stopped_) return false;
109 xmlError* error = xmlCtxtGetLastError(p->context_); 132 xmlError* error = xmlCtxtGetLastError(p->context_);
110 if (error->code == XML_WAR_NS_URI || error->code == XML_WAR_NS_URI_RELATIVE) { 133 if (error->code == XML_WAR_NS_URI || error->code == XML_WAR_NS_URI_RELATIVE) {
111 xmlCtxtResetLastError(p->context_); 134 xmlCtxtResetLastError(p->context_);
@@ -115,4 +138,9 @@ bool LibXMLParser::parse(const std::string& data, bool finalData) {
115 return false; 138 return false;
116} 139}
117 140
141void LibXMLParser::stopParser() {
142 stopped_ = true;
143 xmlStopParser(p->context_);
144}
145
118} 146}
diff --git a/Swiften/Parser/LibXMLParser.h b/Swiften/Parser/LibXMLParser.h
index a863867..e21770d 100644
--- a/Swiften/Parser/LibXMLParser.h
+++ b/Swiften/Parser/LibXMLParser.h
@@ -19,13 +19,16 @@ namespace Swift {
19 */ 19 */
20 class LibXMLParser : public XMLParser, public boost::noncopyable { 20 class LibXMLParser : public XMLParser, public boost::noncopyable {
21 public: 21 public:
22 LibXMLParser(XMLParserClient* client); 22 LibXMLParser(XMLParserClient* client, bool allowComments = false);
23 virtual ~LibXMLParser(); 23 virtual ~LibXMLParser();
24 24
25 bool parse(const std::string& data, bool finalData = false); 25 bool parse(const std::string& data, bool finalData = false);
26 26
27 void stopParser();
28
27 private: 29 private:
28 static bool initialized; 30 static bool initialized;
31 bool stopped_ = false;
29 32
30 struct Private; 33 struct Private;
31 const std::unique_ptr<Private> p; 34 const std::unique_ptr<Private> p;
diff --git a/Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h b/Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h
index dcdbffa..8f9e0e1 100644
--- a/Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h
+++ b/Swiften/Parser/PayloadParsers/UnitTest/PayloadsParserTester.h
@@ -19,7 +19,7 @@ namespace Swift {
19 class PayloadsParserTester : public XMLParserClient { 19 class PayloadsParserTester : public XMLParserClient {
20 public: 20 public:
21 PayloadsParserTester() : level(0) { 21 PayloadsParserTester() : level(0) {
22 xmlParser = PlatformXMLParserFactory().createXMLParser(this); 22 xmlParser = PlatformXMLParserFactory().createXMLParser(this, false);
23 } 23 }
24 24
25 bool parse(const std::string& data) { 25 bool parse(const std::string& data) {
diff --git a/Swiften/Parser/PlatformXMLParserFactory.cpp b/Swiften/Parser/PlatformXMLParserFactory.cpp
index bf66734..a424aca 100644
--- a/Swiften/Parser/PlatformXMLParserFactory.cpp
+++ b/Swiften/Parser/PlatformXMLParserFactory.cpp
@@ -20,11 +20,11 @@ namespace Swift {
20PlatformXMLParserFactory::PlatformXMLParserFactory() { 20PlatformXMLParserFactory::PlatformXMLParserFactory() {
21} 21}
22 22
23std::unique_ptr<XMLParser> PlatformXMLParserFactory::createXMLParser(XMLParserClient* client) { 23std::unique_ptr<XMLParser> PlatformXMLParserFactory::createXMLParser(XMLParserClient* client, bool allowComments) {
24#ifdef HAVE_LIBXML 24#ifdef HAVE_LIBXML
25 return std::make_unique<LibXMLParser>(client); 25 return std::make_unique<LibXMLParser>(client, allowComments);
26#else 26#else
27 return std::make_unique<ExpatParser>(client); 27 return std::make_unique<ExpatParser>(client, allowComments);
28#endif 28#endif
29} 29}
30 30
diff --git a/Swiften/Parser/PlatformXMLParserFactory.h b/Swiften/Parser/PlatformXMLParserFactory.h
index fa3ca19..d72a513 100644
--- a/Swiften/Parser/PlatformXMLParserFactory.h
+++ b/Swiften/Parser/PlatformXMLParserFactory.h
@@ -14,6 +14,6 @@ namespace Swift {
14 public: 14 public:
15 PlatformXMLParserFactory(); 15 PlatformXMLParserFactory();
16 16
17 virtual std::unique_ptr<XMLParser> createXMLParser(XMLParserClient*); 17 virtual std::unique_ptr<XMLParser> createXMLParser(XMLParserClient*, bool allowComments = false);
18 }; 18 };
19} 19}
diff --git a/Swiften/Parser/UnitTest/XMLParserTest.cpp b/Swiften/Parser/UnitTest/XMLParserTest.cpp
index c026b4b..63d30ea 100644
--- a/Swiften/Parser/UnitTest/XMLParserTest.cpp
+++ b/Swiften/Parser/UnitTest/XMLParserTest.cpp
@@ -39,6 +39,10 @@ class XMLParserTest : public CppUnit::TestFixture {
39 CPPUNIT_TEST(testParse_InternalEntity); 39 CPPUNIT_TEST(testParse_InternalEntity);
40 //CPPUNIT_TEST(testParse_UndefinedPrefix); 40 //CPPUNIT_TEST(testParse_UndefinedPrefix);
41 //CPPUNIT_TEST(testParse_UndefinedAttributePrefix); 41 //CPPUNIT_TEST(testParse_UndefinedAttributePrefix);
42 CPPUNIT_TEST(testParse_AllowCommentsInXML);
43 CPPUNIT_TEST(testParse_DisallowCommentsInXML);
44 CPPUNIT_TEST(testParse_Doctype);
45 CPPUNIT_TEST(testParse_ProcessingInstructions);
42 CPPUNIT_TEST_SUITE_END(); 46 CPPUNIT_TEST_SUITE_END();
43 47
44 public: 48 public:
@@ -185,7 +189,7 @@ class XMLParserTest : public CppUnit::TestFixture {
185 } 189 }
186 190
187 void testParse_UnhandledXML() { 191 void testParse_UnhandledXML() {
188 ParserType testling(&client_); 192 ParserType testling(&client_, true);
189 193
190 CPPUNIT_ASSERT(testling.parse("<iq><!-- Testing --></iq>")); 194 CPPUNIT_ASSERT(testling.parse("<iq><!-- Testing --></iq>"));
191 195
@@ -331,13 +335,44 @@ class XMLParserTest : public CppUnit::TestFixture {
331 void testParse_UndefinedAttributePrefix() { 335 void testParse_UndefinedAttributePrefix() {
332 ParserType testling(&client_); 336 ParserType testling(&client_);
333 337
334 CPPUNIT_ASSERT(testling.parse( 338 CPPUNIT_ASSERT(testling.parse("<foo bar:baz='bla'/>"));
335 "<foo bar:baz='bla'/>"));
336 339
337 CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), client_.events[0].attributes.getEntries().size()); 340 CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), client_.events[0].attributes.getEntries().size());
338 CPPUNIT_ASSERT_EQUAL(std::string("bar:baz"), client_.events[0].attributes.getEntries()[0].getAttribute().getName()); 341 CPPUNIT_ASSERT_EQUAL(std::string("bar:baz"), client_.events[0].attributes.getEntries()[0].getAttribute().getName());
339 } 342 }
340 343
344 void testParse_AllowCommentsInXML() {
345 ParserType testling(&client_, true);
346
347 CPPUNIT_ASSERT(testling.parse("<message><!-- Some More Comments Testing --></message>"));
348
349 CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(2), client_.events.size());
350
351 CPPUNIT_ASSERT_EQUAL(Client::StartElement, client_.events[0].type);
352 CPPUNIT_ASSERT_EQUAL(std::string("message"), client_.events[0].data);
353
354 CPPUNIT_ASSERT_EQUAL(Client::EndElement, client_.events[1].type);
355 CPPUNIT_ASSERT_EQUAL(std::string("message"), client_.events[1].data);
356 }
357
358 void testParse_DisallowCommentsInXML() {
359 ParserType testling(&client_);
360
361 CPPUNIT_ASSERT(!testling.parse("<message><!-- Some More Comments Testing --></message>"));
362 }
363
364 void testParse_Doctype() {
365 ParserType testling(&client_);
366
367 CPPUNIT_ASSERT(!testling.parse("<!DOCTYPE greeting SYSTEM \"hello.dtd\">"));
368 }
369
370 void testParse_ProcessingInstructions() {
371 ParserType testling(&client_);
372
373 CPPUNIT_ASSERT(!testling.parse("<?xml-stylesheet type=\"text/xsl\" href=\"Sample.xsl\"?>"));
374 }
375
341 private: 376 private:
342 class Client : public XMLParserClient { 377 class Client : public XMLParserClient {
343 public: 378 public:
@@ -378,6 +413,7 @@ class XMLParserTest : public CppUnit::TestFixture {
378 void handleNamespaceDeclaration(const std::string& prefix, const std::string& uri) override { 413 void handleNamespaceDeclaration(const std::string& prefix, const std::string& uri) override {
379 namespaces_[prefix] = uri; 414 namespaces_[prefix] = uri;
380 } 415 }
416
381 std::vector<Event> events; 417 std::vector<Event> events;
382 private: 418 private:
383 NamespaceMap namespaces_; 419 NamespaceMap namespaces_;
diff --git a/Swiften/Parser/XMLParser.cpp b/Swiften/Parser/XMLParser.cpp
index 8e92fe4..8a0799f 100644
--- a/Swiften/Parser/XMLParser.cpp
+++ b/Swiften/Parser/XMLParser.cpp
@@ -8,7 +8,7 @@
8 8
9namespace Swift { 9namespace Swift {
10 10
11XMLParser::XMLParser(XMLParserClient* client) : client_(client) { 11XMLParser::XMLParser(XMLParserClient* client, bool allowComments) : client_(client), allowComments_(allowComments){
12} 12}
13 13
14XMLParser::~XMLParser() { 14XMLParser::~XMLParser() {
diff --git a/Swiften/Parser/XMLParser.h b/Swiften/Parser/XMLParser.h
index ad79b2d..3b09d22 100644
--- a/Swiften/Parser/XMLParser.h
+++ b/Swiften/Parser/XMLParser.h
@@ -15,7 +15,7 @@ namespace Swift {
15 15
16 class SWIFTEN_API XMLParser { 16 class SWIFTEN_API XMLParser {
17 public: 17 public:
18 XMLParser(XMLParserClient* client); 18 XMLParser(XMLParserClient* client, bool allowComments = false);
19 virtual ~XMLParser(); 19 virtual ~XMLParser();
20 20
21 virtual bool parse(const std::string& data, bool finalData = false) = 0; 21 virtual bool parse(const std::string& data, bool finalData = false) = 0;
@@ -24,7 +24,12 @@ namespace Swift {
24 return client_; 24 return client_;
25 } 25 }
26 26
27 bool allowsComments() const {
28 return allowComments_;
29 }
30
27 private: 31 private:
28 XMLParserClient* client_; 32 XMLParserClient* client_;
33 const bool allowComments_ = false;
29 }; 34 };
30} 35}
diff --git a/Swiften/Parser/XMLParserFactory.h b/Swiften/Parser/XMLParserFactory.h
index 595512b..ae3c90e 100644
--- a/Swiften/Parser/XMLParserFactory.h
+++ b/Swiften/Parser/XMLParserFactory.h
@@ -18,6 +18,6 @@ namespace Swift {
18 public: 18 public:
19 virtual ~XMLParserFactory(); 19 virtual ~XMLParserFactory();
20 20
21 virtual std::unique_ptr<XMLParser> createXMLParser(XMLParserClient*) = 0; 21 virtual std::unique_ptr<XMLParser> createXMLParser(XMLParserClient*, bool allowComments = false) = 0;
22 }; 22 };
23} 23}