And touch up some of its code.
LuaMilkshake reports that it is possible to get
Murmur to emit HTML even when allowhtml is false.
For example via "<br /><b>hello world</>",
which would be translated to "\n<b>hello world</b>"
by the previous HTML filtering code.
Mumble uses QXmlStreamReader to get each token of
an XML document to sanitize it.
Unfortunately, QXmlStreamReader isn't just a tokenizer.
It has a lot of behind-the-scenes XML logic to handle
things such as entities.
Entities definitions are read and handled by the
QXmlStreamReader itself, and are not emitted to a
consumer of QXmlStreamReader. This means that when
QXmlStreamReader gives you a piece of 'text' data,
it will have already translated any entities in the
text and translated them accordingly.
This is why LuaMilkshake's example works.
It is seemingly not possible to get QXmlStreamReader
to avoid handling entities. But I believe we can at
least stop it from allow new entities to be defined.
The original code already did that -- by wrapping
the input XML in a new root tag "<document>". This
ensures that an internal DTD cannot be defined, and
thus that no new entities can be added.
This limits the number of entities we need to
handle, to the small list of default entities
defined by XML: " & &aps; < >.
To ensure that the HTML filter is at least a little
bit effective against these injection attacks, we
ammend the logic of the HTML filter to attmpt to
strip < and > *after* running it through
the original QXmlStreamReader-based filter.
This ensures that no additional HTML elements
can be added via entities.
