xrCore: Increase robustness of XMLDocument parser

- Initialize `m_bIgnoreMissingEndTagError`
- Pass down `bool fatal` to `ParseFile`. Previously we would still hit
  assertions for invalid `#includes` with `fatal` set to `false`
- Fix potential use of nullptr when `strstr` returns `nullptr`
- Introduce a non failing variant of `r_string` called `try_r_string`
- Fix various problems with the handling of `#include`, remove not needed
  string copy and give feedback on invalid things instead of silently
  ignoring problems.
- Fix an infinite loop in tinyxml when parsing an invalid UTF-8 sequence
- Fix infinite recursion or cyclic includes causing a stack overflow

src/xrCore/FS.cpp

@@ -482,6 +482,27 @@ void IReader::skip_stringZ()
     Pos++;
 };
 
+bool IReader::try_r_string(char* dest, size_t tgt_sz)
+{
+    char* src = (char*)data + Pos;
+    size_t sz = advance_term_string();
+    if (sz >= tgt_sz)
+        return false;
+
+#if defined(XR_PLATFORM_WINDOWS)
+    R_ASSERT(!IsBadReadPtr((void*)src, sz));
+#endif
+
+#ifdef _EDITOR
+    CopyMemory(dest, src, sz);
+#else
+    strncpy_s(dest, tgt_sz, src, sz);
+#endif
+    dest[sz] = 0;
+
+    return true;
+}
+
 //---------------------------------------------------
 // temp stream
 CTempReader::~CTempReader() { xr_free(data); };

src/xrCore/FS.h

@@ -400,6 +400,11 @@ class XRCORE_API IReader : public IReaderBase<IReader>
     void r_stringZ(shared_str& dest);
     void r_stringZ(xr_string& dest);
 
+    // Same as r_string but with the difference that it returns 'false' if the read string is longer than 'tgt_sz' and
+    // 'true' if it is shorter
+    [[nodiscard]]
+    bool try_r_string(char* dest, size_t tgt_sz);
+
 public:
     void close();
 

src/xrCore/XML/XMLDocument.cpp

@@ -5,23 +5,87 @@
 pcstr UI_PATH = UI_PATH_DEFAULT;
 pcstr UI_PATH_WITH_DELIMITER = UI_PATH_DEFAULT_WITH_DELIMITER;
 
-XMLDocument::XMLDocument() : m_xml_file_name(), m_root(nullptr), m_pLocalRoot(nullptr) {}
+XMLDocument::XMLDocument() : m_xml_file_name(), m_root(nullptr), m_pLocalRoot(nullptr), m_bIgnoreMissingEndTagError(false) {}
 
 XMLDocument::~XMLDocument() { ClearInternal(); }
 
 void XMLDocument::ClearInternal() { m_Doc.Clear(); }
 
-void ParseFile(pcstr path, CMemoryWriter& W, IReader* F, XMLDocument* xml)
+enum class ParseIncludeResult
 {
-    string4096 str;
+    Success,   /// There is a valid #include and 'out_include_name' returns the filename
+    Error,     /// There is a #include but there is some problem
+    NoInclude, /// There is no #include on this line
+};
+
+// Given a string of the form: '#include "filename"' we return the filename in 'out_include_name'
+ParseIncludeResult ParseInclude(pstr string, pcstr& out_include_name)
+{
+    // Skip any whitespace characters
+    while (*string != '\0' && std::isblank(*string))
+    {
+        ++string;
+    }
+
+    // Check for #include
+    static constexpr pcstr IncludeTag = "#include";
+    if (std::strncmp(string, IncludeTag, 8) != 0)
+        return ParseIncludeResult::NoInclude;
+
+    string += 8;
+
+    // Skip any whitespace characters
+    while (*string != '\0' && std::isblank(*string))
+        ++string;
+
+    // Check that after the tag there is a quote
+    if (*string != '\"')
+        return ParseIncludeResult::Error;
+
+    // Mark the start of the include name
+    ++string;
+    out_include_name = string;
+
+    while (*string != '\0' && *string != '\"')
+        ++string;
+
+    // Check for unterminated or empty include name
+    if (*string == '\0' || out_include_name == string)
+        return ParseIncludeResult::Error;
+
+    // Check for unreasonably long include names
+    const size_t size = string - out_include_name;
+    if (size > 1024)
+        return ParseIncludeResult::Error;
+
+    // NOTE(Andre): Yes this might look scary but it's perfectly fine. Since the include name is already in the string
+    // we are parsing and its not used afterwards we simply replace the closing quote with a null byte and we have a
+    // valid c-string pointed to by 'out_include_name' and safe ourselves the need to copy the string.
+    *string = '\0';
+
+    return ParseIncludeResult::Success;
+}
+
+void ParseFile(pcstr path, CMemoryWriter& W, IReader* F, XMLDocument* xml, bool fatal, u8 include_depth)
+{
+    // Prevent stack overflow due to recursive or cyclic includes
+    if (include_depth >= 128)
+    {
+        R_ASSERT3(!fatal, "XML file[%s] parsing failed. Maximum include depth reached (> 128)", path);
+        Msg("! XML file[%s] parsing failed. Maximum include depth reached (> 128)", path);
+        return;
+    }
 
     const auto tryOpenFile = [&](IReader*& file, pcstr includeName, pcstr comparePath, pcstr uiPath, pcstr uiPathDelim)
     {
         if (file)
             return;
         if (includeName == strstr(includeName, comparePath))
         {
-            shared_str fn = xml->correct_file_name(uiPath, strchr(includeName, _DELIMITER) + 1);
+            pcstr fileName = strstr(includeName, comparePath);
+            fileName = fileName ? ++fileName : includeName;
+
+            shared_str fn = xml->correct_file_name(uiPath, fileName);
             string_path buff;
             strconcat(buff, uiPathDelim, fn.c_str());
             file = FS.r_open(path, buff);
@@ -30,17 +94,18 @@ void ParseFile(pcstr path, CMemoryWriter& W, IReader* F, XMLDocument* xml)
 
     while (!F->eof())
     {
-        F->r_string(str, sizeof str);
-
-        // Skip any spaces or tabs
-        pcstr begin_of_include = str;
-        while (*begin_of_include != '\0' && std::isblank(*begin_of_include))
-            ++begin_of_include;
+        string4096 str;
+        if (!F->try_r_string(str, sizeof(str)))
+        {
+            R_ASSERT3(!fatal, "XML file[%s] parsing failed. Line is too long (>= 4096)", path);
+            Msg("! XML file[%s] parsing failed. Line is too long (>= 4096)", path);
+            return;
+        }
 
-        if (std::strncmp(begin_of_include, "#include", 8) == 0)
+        pcstr inc_name;
+        switch (ParseInclude(str, inc_name))
         {
-            string256 inc_name;
-            if (_GetItem(begin_of_include, 1, inc_name, '"'))
+            case ParseIncludeResult::Success:
             {
                 IReader* I = nullptr;
                 tryOpenFile(I, inc_name, UI_PATH, UI_PATH, UI_PATH_WITH_DELIMITER);
@@ -51,13 +116,26 @@ void ParseFile(pcstr path, CMemoryWriter& W, IReader* F, XMLDocument* xml)
                     I = FS.r_open(path, inc_name);
 
                 if (!I)
-                    FATAL_F("XML file[%s] parsing failed. Can't find include file: [%s]", path, inc_name);
-                ParseFile(path, W, I, xml);
+                {
+                    R_ASSERT4(!fatal, "XML file[%s] parsing failed. Can't find include file: [%s]", path, inc_name);
+                    Msg("! XML file[%s] parsing failed. Can't find include file: [%s]", path, inc_name);
+                    return;
+                }
+
+                ParseFile(path, W, I, xml, fatal, include_depth + 1);
                 FS.r_close(I);
+                break;
             }
+
+            case ParseIncludeResult::Error:
+                R_ASSERT4(!fatal, "XML file[%s] invalid include directive: '%s'", path, str);
+                Msg("! XML file[%s] invalid include directive: '%s'", path, str);
+                break;
+
+            case ParseIncludeResult::NoInclude:
+                W.w_string(str);
+                break;
         }
-        else
-            W.w_string(str);
     }
 }
 
@@ -98,7 +176,7 @@ bool XMLDocument::Load(pcstr path, pcstr xml_filename, bool fatal)
     xr_strcpy(m_xml_file_name, xml_filename);
 
     CMemoryWriter W;
-    ParseFile(path, W, F, this);
+    ParseFile(path, W, F, this, fatal, 0);
     W.w_stringZ("");
     FS.r_close(F);
 

src/xrCore/XML/tinyxmlparser.cpp

@@ -270,6 +270,11 @@ void TiXmlParsingData::Stamp(const char* now, TiXmlEncoding encoding)
                         ++col;
                     } // A normal character.
                 }
+                else
+                {
+                    ++p;
+                    ++col;
+                }
             }
             else
             {