From 7d3ce62ee9842c693c4c8a58e6c50c1523b995fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Sat, 3 Dec 2022 15:44:33 +0100 Subject: [PATCH] improved errorhandling related to deserializing `ErrorMessage` (#4604) --- cli/processexecutor.cpp | 5 +++- lib/errorlogger.cpp | 31 ++++++++++++++++++---- test/testerrorlogger.cpp | 55 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/cli/processexecutor.cpp b/cli/processexecutor.cpp index 707a9c6d1..5cbed00a1 100644 --- a/cli/processexecutor.cpp +++ b/cli/processexecutor.cpp @@ -162,7 +162,10 @@ int ProcessExecutor::handleRead(int rpipe, unsigned int &result) } else if (type == PipeWriter::REPORT_ERROR || type == PipeWriter::REPORT_INFO) { ErrorMessage msg; try { - msg.deserialize(buf); + if (!msg.deserialize(buf)) { + std::cerr << "#### ThreadExecutor::handleRead error" << std::endl; + std::exit(EXIT_FAILURE); + } } catch (const InternalError& e) { std::cerr << "#### ThreadExecutor::handleRead error, internal error:" << e.errorMessage << std::endl; std::exit(EXIT_FAILURE); diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 1f360cd90..a69c6f356 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -258,8 +258,10 @@ std::string ErrorMessage::serialize() const bool ErrorMessage::deserialize(const std::string &data) { + // TODO: clear all fields certainty = Certainty::normal; callStack.clear(); + std::istringstream iss(data); std::array results; std::size_t elem = 0; @@ -268,13 +270,21 @@ bool ErrorMessage::deserialize(const std::string &data) if (!(iss >> len)) return false; - iss.get(); + if (iss.get() != ' ') + return false; + + if (!iss.good()) + return false; + std::string temp; for (unsigned int i = 0; i < len && iss.good(); ++i) { const char c = static_cast(iss.get()); temp.append(1, c); } + if (!iss.good()) + return false; + if (temp == "inconclusive") { certainty = Certainty::inconclusive; continue; @@ -283,14 +293,19 @@ bool ErrorMessage::deserialize(const std::string &data) results[elem++] = temp; } + if (!iss.good()) + return false; + if (elem != 7) - throw InternalError(nullptr, "Internal Error: Deserialization of error message failed"); + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - insufficient elements"); id = results[0]; severity = Severity::fromString(results[1]); - std::istringstream(results[2]) >> cwe.id; - std::istringstream(results[3]) >> hash; - std::istringstream(results[4]) >> file0; + if (!(std::istringstream(results[2]) >> cwe.id)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid CWE ID"); + if (!(std::istringstream(results[3]) >> hash)) + throw InternalError(nullptr, "Internal Error: Deserialization of error message failed - invalid hash"); + file0 = results[4]; mShortMessage = results[5]; mVerboseMessage = results[6]; @@ -298,6 +313,12 @@ bool ErrorMessage::deserialize(const std::string &data) if (!(iss >> stackSize)) return false; + if (iss.get() != ' ') + return false; + + if (stackSize == 0) + return true; + while (iss.good()) { unsigned int len = 0; if (!(iss >> len)) diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 0282fb430..10578d5f7 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -297,7 +297,7 @@ private: "0 ", msg.serialize()); ErrorMessage msg2; - msg2.deserialize(msg.serialize()); + ASSERT(msg2.deserialize(msg.serialize())); ASSERT_EQUALS("errorId", msg2.id); ASSERT_EQUALS(Severity::error, msg2.severity); ASSERT_EQUALS("test.cpp", msg2.file0); @@ -307,8 +307,53 @@ private: } void DeserializeInvalidInput() const { - ErrorMessage msg; - ASSERT_THROW(msg.deserialize("500foobar"), InternalError); + { + // missing/invalid length + // missing separator + ErrorMessage msg; + ASSERT(!msg.deserialize("500foobar")); + } + { + // invalid length + ErrorMessage msg; + ASSERT(!msg.deserialize("foo foobar")); + } + { + // mismatching length + ErrorMessage msg; + ASSERT(!msg.deserialize("8 errorId")); + } + { + // incomplete message + ErrorMessage msg; + ASSERT(!msg.deserialize("7 errorId")); + } + { + // invalid CWE ID + const char str[] = "7 errorId" + "5 error" + "7 invalid" + "1 0" + "8 test.cpp" + "17 Programming error" + "17 Programming error" + "0 "; + ErrorMessage msg; + ASSERT_THROW(msg.deserialize(str), InternalError); + } + { + // invalid hash + const char str[] = "7 errorId" + "5 error" + "1 0" + "7 invalid" + "8 test.cpp" + "17 Programming error" + "17 Programming error" + "0 "; + ErrorMessage msg; + ASSERT_THROW(msg.deserialize(str), InternalError); + } } void SerializeSanitize() const { @@ -326,7 +371,7 @@ private: "0 ", msg.serialize()); ErrorMessage msg2; - msg2.deserialize(msg.serialize()); + ASSERT(msg2.deserialize(msg.serialize())); ASSERT_EQUALS("errorId", msg2.id); ASSERT_EQUALS(Severity::error, msg2.severity); ASSERT_EQUALS("1.c", msg2.file0); @@ -344,7 +389,7 @@ private: ErrorMessage msg(locs, emptyString, Severity::error, "Programming error", "errorId", Certainty::inconclusive); ErrorMessage msg2; - msg2.deserialize(msg.serialize()); + ASSERT(msg2.deserialize(msg.serialize())); ASSERT_EQUALS("[]:;,()", msg2.callStack.front().getfile(false)); ASSERT_EQUALS(":/,;", msg2.callStack.front().getOrigFile(false)); ASSERT_EQUALS(654, msg2.callStack.front().line);