diff options
| -rw-r--r-- | debian/changelog | 3 | ||||
| -rw-r--r-- | storage.cpp | 2 | ||||
| -rw-r--r-- | tests/test-whiteboard.cpp | 31 | ||||
| -rw-r--r-- | whiteboard.cpp | 22 |
4 files changed, 56 insertions, 2 deletions
diff --git a/debian/changelog b/debian/changelog index 41b4ce4..ca9c23f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,8 +3,9 @@ whiteboard (1.8) UNRELEASED; urgency=medium * Added config.maxconnections, defaults to 1000 * Fixed package dependencies for PDF generation * Touch documents on read (i.e. reset timeout on read) + * Validate id, server-side - -- Roland Reichwein <mail@reichwein.it> Fri, 10 Feb 2023 19:18:16 +0100 + -- Roland Reichwein <mail@reichwein.it> Sun, 12 Feb 2023 13:34:47 +0100 whiteboard (1.7) unstable; urgency=medium diff --git a/storage.cpp b/storage.cpp index 6060caf..92f274f 100644 --- a/storage.cpp +++ b/storage.cpp @@ -14,7 +14,7 @@ Storage::Storage(const Config& config): m_db(config.getDataPath() + "/whiteboard.db3", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE), m_maxage(config.getMaxage()), - // Note about VARCHAR(N): "SQLite does not impose any length restrictions" + // Note about VARCHAR(N): "SQLite does not impose any length restrictions" - handled elsewhere in application m_stmt_create(m_db, "CREATE TABLE IF NOT EXISTS documents (id VARCHAR(16) PRIMARY KEY, value BLOB, rev INTEGER, cursorpos INTEGER, timestamp BIGINT)"), m_stmt_getNumberOfDocuments(m_db, "SELECT COUNT(*) FROM documents"), m_stmt_cleanup(m_db, "DELETE FROM documents WHERE timestamp + ? < ?"), diff --git a/tests/test-whiteboard.cpp b/tests/test-whiteboard.cpp index b6fe9c5..3f70bcf 100644 --- a/tests/test-whiteboard.cpp +++ b/tests/test-whiteboard.cpp @@ -222,3 +222,34 @@ TEST_F(WhiteboardTest, max_connections) ASSERT_THROW(WebsocketClient wc4, std::exception); } + +TEST_F(WhiteboardTest, id) +{ + WebsocketClient wc; + + wc.write("<request><command>getfile</command><id>1</id></request>"); + std::string result {wc.read()}; + + EXPECT_EQ(result, "<serverinfo><type>getfile</type><data/><revision>0</revision><pos>0</pos></serverinfo>"); + + wc.write("<request><command>getfile</command><id></id></request>"); + result = wc.read(); + EXPECT_EQ(result, "<serverinfo><type>error</type><message>Message handling error: Invalid id (empty)</message></serverinfo>"); + + wc.write("<request><command>getfile</command><id>01234567890123456789</id></request>"); + result = wc.read(); + EXPECT_EQ(result, "<serverinfo><type>error</type><message>Message handling error: Invalid id (size > 16)</message></serverinfo>"); + + wc.write("<request><command>getfile</command><id>X</id></request>"); + result = wc.read(); + EXPECT_EQ(result, "<serverinfo><type>error</type><message>Message handling error: Invalid id char: X</message></serverinfo>"); + + wc.write("<request><command>getfile</command><id>a.</id></request>"); + result = wc.read(); + EXPECT_EQ(result, "<serverinfo><type>error</type><message>Message handling error: Invalid id char: .</message></serverinfo>"); + + wc.write("<request><command>getfile</command><id>a$b</id></request>"); + result = wc.read(); + EXPECT_EQ(result, "<serverinfo><type>error</type><message>Message handling error: Invalid id char: $</message></serverinfo>"); +} + diff --git a/whiteboard.cpp b/whiteboard.cpp index 5424a79..8fb5415 100644 --- a/whiteboard.cpp +++ b/whiteboard.cpp @@ -76,6 +76,8 @@ Whiteboard::Whiteboard() { } +namespace { + pt::ptree make_ptree(const std::initializer_list<std::pair<std::string, std::string>>& key_values) { pt::ptree ptree; @@ -92,6 +94,21 @@ std::string make_xml(const std::initializer_list<std::pair<std::string, std::str return Reichwein::XML::plain_xml(ptree); } +// throws on invalid id +void validate_id(const std::string& id) { + if (!id.size()) + throw std::runtime_error("Invalid id (empty)"); + + if (id.size() > 16) + throw std::runtime_error("Invalid id (size > 16)"); + + for (const auto c: id) { + if (!((c >= '0' && c <= '9') || (c >= 'a' && c <= 'z'))) + throw std::runtime_error("Invalid id char: "s + c); + } +} + +} // namespace class session: public std::enable_shared_from_this<session> { @@ -316,6 +333,7 @@ public: if (command == "modify") { std::string id {xml.get<std::string>("request.id")}; + validate_id(id); int baserev {xml.get<int>("request.baserev")}; if (baserev != m_storage.getRevision(id)) @@ -341,6 +359,7 @@ public: return make_xml({{"type", "modify"}, {"revision", std::to_string(m_storage.getRevision(id)) }}); } else if (command == "cursorpos") { std::string id {xml.get<std::string>("request.id")}; + validate_id(id); int pos {xml.get<int>("request.pos")}; if (m_storage.getCursorPos(id) != pos) { m_storage.setCursorPos(id, pos); @@ -349,6 +368,7 @@ public: return {}; } else if (command == "getfile") { std::string id {xml.get<std::string>("request.id")}; + validate_id(id); std::string filedata; try { @@ -369,6 +389,7 @@ public: }); } else if (command == "getpos") { std::string id {xml.get<std::string>("request.id")}; + validate_id(id); return make_xml({ {"type", "getpos"}, @@ -395,6 +416,7 @@ public: return stats_xml(); } else if (command == "pdf") { std::string id {xml.get<std::string>("request.id")}; + validate_id(id); Reichwein::Tempfile mdFilePath{".md"}; Reichwein::File::setFile(mdFilePath.getPath(), m_storage.getDocument(id)); Reichwein::Tempfile pdfFilePath{".pdf"}; |