nagai/fune
3
0
Fork 0
forked from mirrors/gecko-dev
Code Pull requests Activity

Bug 1746996 - Ensure that storeRawNames is always called. r=peterv

Browse source 
To avoid copying in the common case, expat points directly into the
(internal) input buffer for strings referenced from the tag stack. When
expat unwinds back to the caller, it attempts to call storeRawNames() to
walk the tag stack and copy any such strings into persistent memory
before the caller potentially invokes XML_Parse() again and shuffles the
input buffer, thereby invalidating these references.

Unfortunately, it doesn't do it in all the right places. Because
different parsing states set |processor| to different callbacks (so that
parsing can resume in the right context), there are a number of
non-obvious entry points. In this case, the input stream was chunked so
that parsing paused in middle of processing an internal entity [1], so
|processor| was set to internalEntityProcessor(), which invokes
doContent() but does not call storeRawNames(). The doContent() call then
parsed some nested tags from the entity. Tags within an entity are
generally required to be balanced, but a host callback returned an error
code to interrupt parsing midway through. This caused Expat to return to
the caller with a tag stack still referencing the input buffer, which
got clobbered in the next call to XML_Parse, causing a tag mismatch on
the next close tag.

Conceptually, this optimization should be managed by doContent(), and I
believe the only reason that isn't the case is that doContent() has so
many return paths (and we don't have RAII in C). We can fix this by
wrapping doContent() in a helper.

[1] &certerror.expiredCert.whatCanYouDoAboutIt2;

Differential Revision: https://phabricator.services.mozilla.com/D134878
This commit is contained in:
Bobby Holley 2022-01-01 05:00:45 +00:00
parent bdc8d80577
commit 65ffce9728
1 changed files with 26 additions and 14 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

40
parser/expat/lib/xmlparse.c
View file

@ -350,6 +350,10 @@ doContent(XML_Parser parser, int startTagLevel, const ENCODING *enc,
const char *start, const char *end, const char **endPtr,
XML_Bool haveMore);
static enum XML_Error
doContentInternal(XML_Parser parser, int startTagLevel, const ENCODING *enc,
const char *start, const char *end, const char **endPtr,
XML_Bool haveMore);
static enum XML_Error
doCdataSection(XML_Parser parser, const ENCODING *, const char **startPtr,
const char *end, const char **nextPtr, XML_Bool haveMore);
#ifdef XML_DTD
@ -2525,13 +2529,8 @@ contentProcessor(XML_Parser parser,
const char *end,
const char **endPtr)
{
enum XML_Error result = doContent(parser, 0, encoding, start, end,
endPtr, (XML_Bool)!ps_finalBuffer);
if (result == XML_ERROR_NONE) {
if (!storeRawNames(parser))
return XML_ERROR_NO_MEMORY;
}
return result;
return doContent(parser, 0, encoding, start, end,
endPtr, (XML_Bool)!ps_finalBuffer);
}
static enum XML_Error PTRCALL
@ -2641,13 +2640,8 @@ externalEntityContentProcessor(XML_Parser parser,
const char *end,
const char **endPtr)
{
enum XML_Error result = doContent(parser, 1, encoding, start, end,
endPtr, (XML_Bool)!ps_finalBuffer);
if (result == XML_ERROR_NONE) {
if (!storeRawNames(parser))
return XML_ERROR_NO_MEMORY;
}
return result;
return doContent(parser, 1, encoding, start, end,
endPtr, (XML_Bool)!ps_finalBuffer);
}
static enum XML_Error
@ -2658,6 +2652,24 @@ doContent(XML_Parser parser,
const char *end,
const char **nextPtr,
XML_Bool haveMore)
{
enum XML_Error result = doContentInternal(parser, startTagLevel, enc,
s, end, nextPtr, haveMore);
if (result == XML_ERROR_NONE) {
if (!storeRawNames(parser))
return XML_ERROR_NO_MEMORY;
}
return result;
}
static enum XML_Error
doContentInternal(XML_Parser parser,
int startTagLevel,
const ENCODING *enc,
const char *s,
const char *end,
const char **nextPtr,
XML_Bool haveMore)
{
/* save one level of indirection */
DTD * const dtd = _dtd;