Validate ids, server-side

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 &gt; 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"};