From 93fed19c6ad9065563c1af72c1240583b1af5d0b Mon Sep 17 00:00:00 2001 From: elexis Date: Tue, 17 Sep 2019 23:00:36 +0000 Subject: [PATCH] Reliably report and reject invalid XML files following a18fbd12ec, refs #245, fixes #5222. Don't write XMB files for XML files that failed the validation, otherwise the XML validation error will not be reported on consecutive program starts anymore (as the XMB is loaded, skipping validation). This had resulted in invalid XML going unnoticed and committed in credentials.xml in 80dbd1f2a3. Differential Revision: https://code.wildfiregames.com/D1574 Reported by: gameboy Comments By: bb Tested on: clang 8.0.1, Jenkins This was SVN commit r22921. --- source/ps/Errors.cpp | 10 ++++++++++ source/ps/XML/Xeromyces.cpp | 11 ++++++++--- source/ps/XML/Xeromyces.h | 1 + 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/source/ps/Errors.cpp b/source/ps/Errors.cpp index 43055ce0f5..bea2c86260 100644 --- a/source/ps/Errors.cpp +++ b/source/ps/Errors.cpp @@ -61,6 +61,7 @@ class PSERROR_System_SDLInitFailed : public PSERROR_System { public: PSERROR_Sys class PSERROR_System_VmodeFailed : public PSERROR_System { public: PSERROR_System_VmodeFailed(); PSERROR_System_VmodeFailed(const char* msg); PSRETURN getCode() const; }; class PSERROR_Xeromyces_XMLOpenFailed : public PSERROR_Xeromyces { public: PSERROR_Xeromyces_XMLOpenFailed(); PSERROR_Xeromyces_XMLOpenFailed(const char* msg); PSRETURN getCode() const; }; class PSERROR_Xeromyces_XMLParseError : public PSERROR_Xeromyces { public: PSERROR_Xeromyces_XMLParseError(); PSERROR_Xeromyces_XMLParseError(const char* msg); PSRETURN getCode() const; }; +class PSERROR_Xeromyces_XMLValidationFailed : public PSERROR_Xeromyces { public: PSERROR_Xeromyces_XMLValidationFailed(); PSERROR_Xeromyces_XMLValidationFailed(const char* msg); PSRETURN getCode() const; }; extern const PSRETURN PSRETURN_CVFSFile_AlreadyLoaded = 0x01000001; extern const PSRETURN PSRETURN_CVFSFile_LoadFailed = 0x01000002; @@ -103,6 +104,7 @@ extern const PSRETURN PSRETURN_System_SDLInitFailed = 0x0a000002; extern const PSRETURN PSRETURN_System_VmodeFailed = 0x0a000003; extern const PSRETURN PSRETURN_Xeromyces_XMLOpenFailed = 0x0b000001; extern const PSRETURN PSRETURN_Xeromyces_XMLParseError = 0x0b000002; +extern const PSRETURN PSRETURN_Xeromyces_XMLValidationFailed = 0x0b000003; extern const PSRETURN MASK__PSRETURN_CVFSFile = 0xff000000; extern const PSRETURN CODE__PSRETURN_CVFSFile = 0x01000000; @@ -215,6 +217,8 @@ extern const PSRETURN MASK__PSRETURN_Xeromyces_XMLOpenFailed = 0xffffffff; extern const PSRETURN CODE__PSRETURN_Xeromyces_XMLOpenFailed = 0x0b000001; extern const PSRETURN MASK__PSRETURN_Xeromyces_XMLParseError = 0xffffffff; extern const PSRETURN CODE__PSRETURN_Xeromyces_XMLParseError = 0x0b000002; +extern const PSRETURN MASK__PSRETURN_Xeromyces_XMLValidationFailed = 0xffffffff; +extern const PSRETURN CODE__PSRETURN_Xeromyces_XMLValidationFailed = 0x0b000003; PSERROR_CVFSFile::PSERROR_CVFSFile(const char* msg) : PSERROR(msg) { } PSERROR_Deserialize::PSERROR_Deserialize(const char* msg) : PSERROR(msg) { } @@ -395,6 +399,10 @@ PSERROR_Xeromyces_XMLParseError::PSERROR_Xeromyces_XMLParseError() : PSERROR_Xer PSERROR_Xeromyces_XMLParseError::PSERROR_Xeromyces_XMLParseError(const char* msg) : PSERROR_Xeromyces(msg) { } PSRETURN PSERROR_Xeromyces_XMLParseError::getCode() const { return 0x0b000002; } +PSERROR_Xeromyces_XMLValidationFailed::PSERROR_Xeromyces_XMLValidationFailed() : PSERROR_Xeromyces(NULL) { } +PSERROR_Xeromyces_XMLValidationFailed::PSERROR_Xeromyces_XMLValidationFailed(const char* msg) : PSERROR_Xeromyces(msg) { } +PSRETURN PSERROR_Xeromyces_XMLValidationFailed::getCode() const { return 0x0b000003; } + PSERROR::PSERROR(const char* msg) : m_msg(msg) { } @@ -448,6 +456,7 @@ const char* GetErrorString(PSRETURN code) case 0x0a000003: return "System_VmodeFailed"; case 0x0b000001: return "Xeromyces_XMLOpenFailed"; case 0x0b000002: return "Xeromyces_XMLParseError"; + case 0x0b000003: return "Xeromyces_XMLValidationFailed"; default: return "Unrecognised error"; } @@ -498,6 +507,7 @@ void ThrowError(PSRETURN code) case 0x0a000003: throw PSERROR_System_VmodeFailed(); break; case 0x0b000001: throw PSERROR_Xeromyces_XMLOpenFailed(); break; case 0x0b000002: throw PSERROR_Xeromyces_XMLParseError(); break; + case 0x0b000003: throw PSERROR_Xeromyces_XMLValidationFailed(); break; default: throw PSERROR_Error_InvalidError(); // Hmm... } diff --git a/source/ps/XML/Xeromyces.cpp b/source/ps/XML/Xeromyces.cpp index 45a59e5892..b080d4766f 100644 --- a/source/ps/XML/Xeromyces.cpp +++ b/source/ps/XML/Xeromyces.cpp @@ -178,8 +178,10 @@ PSRETURN CXeromyces::ConvertFile(const PIVFS& vfs, const VfsPath& filename, cons std::lock_guard lock(g_ValidatorCacheLock); RelaxNGValidator& validator = GetValidator(validatorName); if (validator.CanValidate() && !validator.ValidateEncoded(doc)) - // For now, log the error and continue, in the future we might fail + { LOGERROR("CXeromyces: failed to validate XML file %s", filename.string8()); + return PSRETURN_Xeromyces_XMLValidationFailed; + } } WriteBuffer writeBuffer; @@ -187,7 +189,8 @@ PSRETURN CXeromyces::ConvertFile(const PIVFS& vfs, const VfsPath& filename, cons xmlFreeDoc(doc); - // Save the file to disk, so it can be loaded quickly next time + // Save the file to disk, so it can be loaded quickly next time. + // Don't save if invalid, because we want the syntax error every program start. vfs->CreateFile(xmbPath, writeBuffer.Data(), writeBuffer.Size()); m_XMBBuffer = writeBuffer.Data(); // add a reference @@ -233,8 +236,10 @@ PSRETURN CXeromyces::LoadString(const char* xml, const std::string& validatorNam std::lock_guard lock(g_ValidatorCacheLock); RelaxNGValidator& validator = GetValidator(validatorName); if (validator.CanValidate() && !validator.ValidateEncoded(doc)) - // For now, log the error and continue, in the future we might fail + { LOGERROR("CXeromyces: failed to validate XML string"); + return PSRETURN_Xeromyces_XMLValidationFailed; + } } WriteBuffer writeBuffer; diff --git a/source/ps/XML/Xeromyces.h b/source/ps/XML/Xeromyces.h index ae573cb2cc..4cc3f7b71e 100644 --- a/source/ps/XML/Xeromyces.h +++ b/source/ps/XML/Xeromyces.h @@ -28,6 +28,7 @@ ERROR_GROUP(Xeromyces); ERROR_TYPE(Xeromyces, XMLOpenFailed); ERROR_TYPE(Xeromyces, XMLParseError); +ERROR_TYPE(Xeromyces, XMLValidationFailed); #include "XeroXMB.h"