diff options
| -rw-r--r-- | spec/util_xml_spec.lua | 34 | ||||
| -rw-r--r-- | util/xml.lua | 25 |
2 files changed, 56 insertions, 3 deletions
diff --git a/spec/util_xml_spec.lua b/spec/util_xml_spec.lua index 11820894..28a1cea7 100644 --- a/spec/util_xml_spec.lua +++ b/spec/util_xml_spec.lua @@ -12,9 +12,41 @@ describe("util.xml", function() <a:z/> <!-- prefix 'a' is nil here, but should be 'b' --> </x> ]] - local stanza = xml.parse(x); + local stanza = xml.parse(x, {allow_comments = true}); assert.are.equal(stanza.tags[2].attr.xmlns, "b"); assert.are.equal(stanza.tags[2].namespaces["a"], "b"); end); + + it("should reject doctypes", function() + local x = "<!DOCTYPE foo []><foo/>"; + local ok = xml.parse(x); + assert.falsy(ok); + end); + + it("should reject comments by default", function() + local x = "<foo><!-- foo --></foo>"; + local ok = xml.parse(x); + assert.falsy(ok); + end); + + it("should allow comments if asked nicely", function() + local x = "<foo><!-- foo --></foo>"; + local stanza = xml.parse(x, {allow_comments = true}); + assert.are.equal(stanza.name, "foo"); + assert.are.equal(#stanza, 0); + end); + + it("should reject processing instructions", function() + local x = "<foo><?php die(); ?></foo>"; + local ok = xml.parse(x); + assert.falsy(ok); + end); + + it("should allow an xml declaration", function() + local x = "<?xml version='1.0'?><foo/>"; + local stanza = xml.parse(x); + assert.truthy(stanza); + assert.are.equal(stanza.name, "foo"); + end); end); end); diff --git a/util/xml.lua b/util/xml.lua index 38411bfb..549ab104 100644 --- a/util/xml.lua +++ b/util/xml.lua @@ -3,6 +3,7 @@ local st = require "util.stanza"; local lxp = require "lxp"; local t_insert = table.insert; local t_remove = table.remove; +local error = error; local _ENV = nil; -- luacheck: std none @@ -13,7 +14,7 @@ local parse_xml = (function() }; local ns_separator = "\1"; local ns_pattern = "^([^"..ns_separator.."]*)"..ns_separator.."?(.*)$"; - return function(xml) + return function(xml, options) --luacheck: ignore 212/self local handler = {}; local stanza = st.stanza("root"); @@ -64,7 +65,27 @@ local parse_xml = (function() function handler:EndElement() stanza:up(); end - local parser = lxp.new(handler, ns_separator); + local parser; + -- SECURITY: These two handlers, especially the Doctype one, are required to prevent exploits such as Billion Laughs. + function handler:StartDoctypeDecl() + if not parser.stop or not parser:stop() then + error("Failed to abort parsing"); + end + end + function handler:ProcessingInstruction() + if not parser.stop or not parser:stop() then + error("Failed to abort parsing"); + end + end + if not options or not options.allow_comments then + -- NOTE: comments are generally harmless and can be useful when parsing configuration files or other data, even user-provided data + function handler:Comment() + if not parser.stop or not parser:stop() then + error("Failed to abort parsing"); + end + end + end + parser = lxp.new(handler, ns_separator); local ok, err, line, col = parser:parse(xml); if ok then ok, err, line, col = parser:parse(); end --parser:close(); |