<div dir="ltr"><div><div dir="auto">Hi Xapian developers,<br><br>Since Xapian publishes no private security contact, I am writing to the list rather than putting this anywhere more public; my name is Arpit Jain and I work on open-source supply-chain security.<br><br>I think Xapian::MSet::snippet() in xapian-core breaks its documented HTML-escaping guarantee on one fast path: when hi_start and hi_end are both empty and the text already fits inside length, it returns the caller's text byte for byte with no escaping. I built and ran a proof of concept against the released 2.0.0 library; the output is below.<br><br>Summary<br>-------<br>All of the escaping in snippet generation lives in append_escaping_xml(), which is only reached from SnipPipe::drain(). The branch at the top of MSet::Internal::snippet() returns before drain() ever runs, so on that path nothing is escaped, even though the header docs promise unconditionally that the returned text is safe for HTML. The result is inconsistent in a way that is easy to miss: the same call escapes correctly as soon as the text is longer than length, so short documents come back raw and long ones do not.<br><br>Where it is<br>-----------<br>xapian-core/queryparser/termgenerator_internal.cc:836-839, at tag v2.0.0 and on git master:<br><br>    if (hi_start.empty() && hi_end.empty() && text.size() <= length) {<br>        // Too easy!<br>        return string{text};<br>    }<br><br>The guarantee it contradicts is at xapian-core/include/xapian/mset.h:404, in the doc comment for MSet::snippet(): "The returned text is escaped to make it suitable for use in HTML (though beware that in upstream releases 1.4.5 and earlier this escaping was sometimes incomplete)". There is no exception there for empty markers.<br><br>What happens<br>------------<br>An application that indexes untrusted content and renders results with snippet() using empty hi_start and hi_end, trusting the documented escaping, emits the attacker's markup into the page for every document short enough to hit the branch. That is stored XSS in the embedding application, and the attacker needs nothing beyond the normal content-submission path.<br><br>To be straight about the limits: this is a library contract violation rather than a hole in Xapian itself, it only bites callers who pass empty markers and skip their own escaping, and I have not gone hunting for a specific downstream that does that. Omega is unaffected because it uses non-empty markers. What makes it worth fixing anyway is that the documentation tells callers they do not need to escape, and the escaping CVE-2018-0499 added in 2018 sits below the branch that skips it.<br><br>Proof of concept<br>----------------<br>Against xapian-core 2.0.0 (Homebrew bottle, arm64 macOS). The driver uses the public API only: index one document, run a real query through QueryParser and Enquire, then call snippet() three ways on the same MSet. Includes and the main() wrapper elided:<br><br>    Xapian::WritableDatabase db("/tmp/xsnip/db", Xapian::DB_CREATE_OR_OVERWRITE);<br>    Xapian::Stem stemmer("english");<br>    Xapian::TermGenerator tg;<br>    tg.set_stemmer(stemmer);<br>    Xapian::Document doc;<br>    tg.set_document(doc);<br>    std::string text = "<script>alert(1)</script> hello world";<br>    tg.index_text(text);<br>    doc.set_data(text);<br>    db.add_document(doc);<br>    db.commit();<br><br>    Xapian::Enquire enq(db);<br>    Xapian::QueryParser qp;<br>    qp.set_stemmer(stemmer);<br>    qp.set_stemming_strategy(Xapian::QueryParser::STEM_SOME);<br>    enq.set_query(qp.parse_query("hello"));<br>    Xapian::MSet mset = enq.get_mset(0, 10);<br>    unsigned flags = Xapian::MSet::SNIPPET_BACKGROUND_MODEL | Xapian::MSet::SNIPPET_EXHAUSTIVE;<br><br>    std::cout << "version           : " << Xapian::version_string() << "\n";<br>    std::cout << "input             : " << text << "\n";<br>    std::cout << "default markers   : " << mset.snippet(text, 500, stemmer) << "\n";<br>    std::cout << "empty markers     : " << mset.snippet(text, 500, stemmer, flags, "", "") << "\n";<br>    std::cout << "empty, len=20     : " << mset.snippet(text, 20, stemmer, flags, "", "") << "\n";<br><br>Built with g++ -std=c++17 t.cc -o t $(xapian-config --cxxflags --libs), then run. Observed output:<br><br>    version           : 2.0.0<br>    input             : <script>alert(1)</script> hello world<br>    default markers   : &lt;script&gt;alert(1)&lt;/script&gt; <b>hello</b> world<br>    empty markers     : <script>alert(1)</script> hello world<br>    empty, len=20     : ...hello world<br><br>Line 3 is the guarded path and escapes as documented. Line 4 is the same MSet, same text, same flags, empty markers, and the script tag comes back raw. Line 5 is that same empty-marker call with length cut to 20 so the text no longer fits, which skips the branch and escapes again.<br><br>Suggested fix<br>-------------<br>Either drop the fast path so everything goes through SnipPipe::drain(), or keep it and run append_escaping_xml() over text into the return string instead of returning string{text}, which preserves the performance win. If the intent really is that empty markers mean "give me the raw text", I would say so in the mset.h comment, but I would still argue against leaving the code as it stands, because the current rule is not "empty markers mean raw", it is "empty markers mean raw, but only when the text is short enough", and no caller can reason about that.<br><br>Why I don't think it's a duplicate<br>----------------------------------<br>The escaping was added by commit c1986aff, "Add missing XML escaping in MSet::snippet()", whose message is "We were escaping in some cases, but not all". That is the CVE-2018-0499 fix. It introduced append_escaping_xml(), applied it at three sites all inside SnipPipe::drain(), and added a test in tests/api_snippets.cc. It did not touch MSet::Internal::snippet(). The empty-marker branch returns before drain() is ever constructed, so the 2018 fix could not have covered it, and the run above shows it still returns raw text on 2.0.0. This is the leftover "some cases, but not all" case, not a rediscovery of the fixed ones.<br><br>Severity and classification (my read, your call)<br>------------------------------------------------<br>Medium. CVSS:3.1/AV:N/AC:H/PR:L/UI:R/S:C/C:L/I:L/A:N. CWE-116, with CWE-79 as the downstream effect. AC is High because the impact depends on a caller configuration the attacker does not control, and scope is Changed because the consequence lands in the embedding application's browser context rather than in Xapian.<br><br>Affected versions<br>-----------------<br>Verified by reading the source at v1.4.6, v1.4.31, v1.5.2 and v2.0.0, and on git master; the line is at termgenerator_internal.cc:762-765 in v1.4.31. The proof of concept was executed against the released 2.0.0 library. I have not pinned down the first release containing the branch, only that it predates the 2018 escaping fix.<br><br>Tooling<br>-------<br>I found this by starting from CVE-2018-0499, reading what its fix commit actually changed, then looking for paths in snippet() that return without going through drain(). I used AI assistance while investigating, but I wrote and compiled the proof of concept myself and the output above is what it printed against xapian-core 2.0.0, so this is empirically verified rather than inferred from source. If you publish an advisory or request a CVE for this, my GitHub handle is arpitjain099. Happy to send a patch for whichever fix shape you prefer.<br><br>Thanks,<br>Arpit<br></div></div></div>