fix(xmlreader): bound reads for truncated BOM prefixes

libs/xmlreader/src/XML_Reader.cpp

@@ -398,16 +398,21 @@ UTFCode XML_Reader::CheckUTF(const string& text)
   }
 
   const char* str = text.c_str();
+  const char* const end = str + text.size();
+  const auto nextByte = [&str, end]() {
+    // A truncated BOM must not advance the cursor past the string terminator.
+    return str != end ? *str++ : '\0';
+  };
 
   UTFCode utfDetected = UTFCode::UTF_NULL;
   // Keep the byte-decoding switch structure extensible for additional BOM variants.
-  switch (*str++) {
+  switch (nextByte()) {
     case (char)0xEF: {
       // codeql[cpp/trivial-switch]
-      switch (*str++) {
+      switch (nextByte()) {
         case (char)0xBB: {
           // codeql[cpp/trivial-switch]
-          switch (*str++) {
+          switch (nextByte()) {
             case (char)0xBF: {
               utfDetected = UTFCode::UTF8;
             } break;
@@ -422,7 +427,7 @@ UTFCode XML_Reader::CheckUTF(const string& text)
 
     case (char)0xFE: {
       // codeql[cpp/trivial-switch]
-      switch (*str++) {
+      switch (nextByte()) {
         case (char)0xFF: {
           utfDetected = UTFCode::UTF16_BE;
         } break;
@@ -433,7 +438,7 @@ UTFCode XML_Reader::CheckUTF(const string& text)
 
     case (char)0xFF: {
       // codeql[cpp/trivial-switch]
-      switch (*str++) {
+      switch (nextByte()) {
         case (char)0xFE: {
           utfDetected = UTFCode::UTF16_LE;
         } break;

libs/xmlreader/test/XmlReaderTest.cpp

@@ -7,6 +7,8 @@
 #include "gtest/gtest.h"
 #include "XML_Reader.h"
 
+#include <vector>
+
 using namespace std;
 
 static string utfInfo = "한€ह";
@@ -97,6 +99,106 @@ TEST(XmlReaderTest, CheckUTF)
   EXPECT_EQ(XmlTypes::UTFCode::UTF16_LE, XML_Reader::CheckUTF(utfStr));
 }
 
+namespace {
+
+class XmlReaderUTFTest : public testing::Test, public IErrConsumer {
+protected:
+  void SetUp() override {
+    const auto log = ErrLog::Get();
+    m_previousConsumer = log->SetErrConsumer(this);
+    m_previousQuiet = log->IsQuietMode();
+    m_previousErrors = log->GetErrCnt();
+    m_previousWarnings = log->GetWarnCnt();
+    log->SetQuietMode(false);
+    log->ResetMsgCount();
+  }
+
+  void TearDown() override {
+    const auto log = ErrLog::Get();
+    log->SetErrConsumer(m_previousConsumer);
+    log->SetQuietMode(m_previousQuiet);
+    log->ResetMsgCount();
+    for (int i = 0; i < m_previousErrors; ++i) {
+      log->IncErrCnt();
+    }
+    for (int i = 0; i < m_previousWarnings; ++i) {
+      log->IncWarnCnt();
+    }
+  }
+
+  bool Consume(const PdscMsg& msg, const string&) override {
+    m_messages.push_back(msg.GetMsgNum());
+    if (msg.GetMsgNum() == "M412") {
+      m_remainder = msg.GetSubstitute("STR");
+    }
+    return true;
+  }
+
+  vector<string> m_messages;
+  string m_remainder;
+
+private:
+  XML_Reader m_reader{nullptr}; // Register XML diagnostic messages.
+  IErrConsumer* m_previousConsumer = nullptr;
+  bool m_previousQuiet = false;
+  int m_previousErrors = 0;
+  int m_previousWarnings = 0;
+};
+
+TEST_F(XmlReaderUTFTest, TruncatedPreamblesIgnoreRetainedStorage)
+{
+  const string prefixes[] = { "\xEF", "\xFE", "\xFF", "\xEF\xBB" };
+  for (const auto& prefix : prefixes) {
+    SCOPED_TRACE(testing::PrintToString(prefix));
+    string text = prefix + "unused";
+    text.resize(prefix.size()); // Retained bytes beyond the terminator are not input.
+    m_messages.clear();
+
+    EXPECT_EQ(XmlTypes::UTFCode::UTF_NULL, XML_Reader::CheckUTF(text));
+    EXPECT_TRUE(m_messages.empty());
+  }
+}
+
+TEST_F(XmlReaderUTFTest, PreservesPreambleAndRemainderDiagnostics)
+{
+  using UTFCode = XmlTypes::UTFCode;
+  const struct {
+    string text;
+    UTFCode code;
+    vector<string> messages;
+    string remainder;
+  } cases[] = {
+    { "",                    UTFCode::UTF_NULL, {},                       "" },
+    { "x",                   UTFCode::UTF_NULL, {},                       "" },
+    { "\xEF\xBB\xBF",        UTFCode::UTF8,     {"M411"},                 "" },
+    { "\xFE\xFF",            UTFCode::UTF16_BE, {"M411", "M413"},         "" },
+    { "\xFF\xFE",            UTFCode::UTF16_LE, {"M411", "M413"},         "" },
+    { "\xEF\xBB\xBF" "text", UTFCode::UTF8,     {"M411", "M412"},         "text" },
+    { "\xFE\xFF" "text",     UTFCode::UTF16_BE, {"M411", "M413", "M412"}, "text" },
+    { "\xFF\xFE" "text",     UTFCode::UTF16_LE, {"M411", "M413", "M412"}, "text" },
+    { "\xEF!",               UTFCode::UTF_NULL, {},                       "" },
+    { "\xEF\xBB!",           UTFCode::UTF_NULL, {},                       "" },
+    { "\xFE!",               UTFCode::UTF_NULL, {},                       "" },
+    { "\xFF!",               UTFCode::UTF_NULL, {},                       "" },
+    { "\xEF!text",           UTFCode::UTF_NULL, {"M412"},                 "text" },
+    { "\xEF\xBB!text",       UTFCode::UTF_NULL, {"M412"},                 "text" },
+    { "\xFE!text",           UTFCode::UTF_NULL, {"M412"},                 "text" },
+    { "\xFF!text",           UTFCode::UTF_NULL, {"M412"},                 "text" },
+    { "xtext",               UTFCode::UTF_NULL, {"M412"},                 "text" },
+  };
+  for (const auto& test : cases) {
+    SCOPED_TRACE(testing::PrintToString(test.text));
+    m_messages.clear();
+    m_remainder.clear();
+
+    EXPECT_EQ(test.code, XML_Reader::CheckUTF(test.text));
+    EXPECT_EQ(test.messages, m_messages);
+    EXPECT_EQ(test.remainder, m_remainder);
+  }
+}
+
+} // namespace
+
 TEST(XmlReaderTest, ReadAttributes)
 {
   XmlTypes::XmlNode_t node;