From 5ad929104e4db4b788c3792bca3e7d02283dc77e Mon Sep 17 00:00:00 2001
From: Mark Haines <mark.haines@matrix.org>
Date: Fri, 10 Jul 2015 18:00:18 +0100
Subject: [PATCH] Version the pickled objects and check for errors when
unpickling them
---
include/olm/error.hh | 2 ++
src/account.cpp | 12 ++++++++++++
src/olm.cpp | 29 +++++++++++++++++++++++++----
src/session.cpp | 11 +++++++++++
4 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/include/olm/error.hh b/include/olm/error.hh
index 960de72..b0d3764 100644
--- a/include/olm/error.hh
+++ b/include/olm/error.hh
@@ -27,6 +27,8 @@ enum struct ErrorCode {
BAD_MESSAGE_KEY_ID = 6, /*!< The message references an unknown key id */
INVALID_BASE64 = 7, /*!< The input base64 was invalid */
BAD_ACCOUNT_KEY = 8, /*!< The supplied account key is invalid */
+ UNKNOWN_PICKLE_VERSION = 9, /*!< The pickled object is too new */
+ CORRUPTED_PICKLE = 10, /*!< The pickled object couldn't be decoded */
};
} // namespace olm
diff --git a/src/account.cpp b/src/account.cpp
index 796b06e..cf6f0cb 100644
--- a/src/account.cpp
+++ b/src/account.cpp
@@ -360,11 +360,16 @@ static std::uint8_t const * unpickle(
} // namespace olm
+namespace {
+static const std::uint32_t ACCOUNT_PICKLE_VERSION = 1;
+}
+
std::size_t olm::pickle_length(
olm::Account const & value
) {
std::size_t length = 0;
+ length += olm::pickle_length(ACCOUNT_PICKLE_VERSION);
length += olm::pickle_length(value.identity_keys);
length += olm::pickle_length(value.one_time_keys);
length += olm::pickle_length(value.next_one_time_key_id);
@@ -376,6 +381,7 @@ std::uint8_t * olm::pickle(
std::uint8_t * pos,
olm::Account const & value
) {
+ pos = olm::pickle(pos, ACCOUNT_PICKLE_VERSION);
pos = olm::pickle(pos, value.identity_keys);
pos = olm::pickle(pos, value.one_time_keys);
pos = olm::pickle(pos, value.next_one_time_key_id);
@@ -387,6 +393,12 @@ std::uint8_t const * olm::unpickle(
std::uint8_t const * pos, std::uint8_t const * end,
olm::Account & value
) {
+ uint32_t pickle_version;
+ pos = olm::unpickle(pos, end, pickle_version);
+ if (pickle_version != ACCOUNT_PICKLE_VERSION) {
+ value.last_error = olm::ErrorCode::UNKNOWN_PICKLE_VERSION;
+ return end;
+ }
pos = olm::unpickle(pos, end, value.identity_keys);
pos = olm::unpickle(pos, end, value.one_time_keys);
pos = olm::unpickle(pos, end, value.next_one_time_key_id);
diff --git a/src/olm.cpp b/src/olm.cpp
index efb1dfe..f3ce2ae 100644
--- a/src/olm.cpp
+++ b/src/olm.cpp
@@ -151,7 +151,7 @@ std::size_t b64_input(
return raw_length;
}
-const char * errors[9] {
+const char * errors[11] {
"SUCCESS",
"NOT_ENOUGH_RANDOM",
"OUTPUT_BUFFER_TOO_SMALL",
@@ -161,6 +161,8 @@ const char * errors[9] {
"BAD_MESSAGE_KEY_ID",
"INVALID_BASE64",
"BAD_ACCOUNT_KEY",
+ "UNKNOWN_PICKLE_VERSION",
+ "CORRUPTED_PICKLE",
};
} // namespace
@@ -282,7 +284,16 @@ size_t olm_unpickle_account(
return std::size_t(-1);
}
std::uint8_t * const end = pos + raw_length;
- unpickle(pos, end, object);
+ /* On success unpickle will return (pos + raw_length). If unpickling
+ * terminates too soon then it will return a pointer before
+ * (pos + raw_length). On error unpickle will return (pos + raw_length + 1).
+ */
+ if (end != unpickle(pos, end + 1, object)) {
+ if (object.last_error == olm::ErrorCode::SUCCESS) {
+ object.last_error = olm::ErrorCode::CORRUPTED_PICKLE;
+ }
+ return std::size_t(-1);
+ }
return pickled_length;
}
@@ -300,8 +311,18 @@ size_t olm_unpickle_session(
if (raw_length == std::size_t(-1)) {
return std::size_t(-1);
}
- std::uint8_t * const end = pos + raw_length;
- unpickle(pos, end, object);
+
+ std::uint8_t * const end = pos + raw_length + 1;
+ /* On success unpickle will return (pos + raw_length). If unpickling
+ * terminates too soon then it will return a pointer before
+ * (pos + raw_length). On error unpickle will return (pos + raw_length + 1).
+ */
+ if (end != unpickle(pos, end + 1, object)) {
+ if (object.last_error == olm::ErrorCode::SUCCESS) {
+ object.last_error = olm::ErrorCode::CORRUPTED_PICKLE;
+ }
+ return std::size_t(-1);
+ }
return pickled_length;
}
diff --git a/src/session.cpp b/src/session.cpp
index 4abf6cf..654cf1f 100644
--- a/src/session.cpp
+++ b/src/session.cpp
@@ -353,11 +353,15 @@ std::size_t olm::Session::decrypt(
return result;
}
+namespace {
+static const std::uint32_t SESSION_PICKLE_VERSION = 1;
+}
std::size_t olm::pickle_length(
Session const & value
) {
std::size_t length = 0;
+ length += olm::pickle_length(SESSION_PICKLE_VERSION);
length += olm::pickle_length(value.received_message);
length += olm::pickle_length(value.alice_identity_key);
length += olm::pickle_length(value.alice_base_key);
@@ -371,6 +375,7 @@ std::uint8_t * olm::pickle(
std::uint8_t * pos,
Session const & value
) {
+ pos = olm::pickle(pos, SESSION_PICKLE_VERSION);
pos = olm::pickle(pos, value.received_message);
pos = olm::pickle(pos, value.alice_identity_key);
pos = olm::pickle(pos, value.alice_base_key);
@@ -384,6 +389,12 @@ std::uint8_t const * olm::unpickle(
std::uint8_t const * pos, std::uint8_t const * end,
Session & value
) {
+ uint32_t pickle_version;
+ pos = olm::unpickle(pos, end, pickle_version);
+ if (pickle_version != SESSION_PICKLE_VERSION) {
+ value.last_error = olm::ErrorCode::UNKNOWN_PICKLE_VERSION;
+ return end;
+ }
pos = olm::unpickle(pos, end, value.received_message);
pos = olm::unpickle(pos, end, value.alice_identity_key);
pos = olm::unpickle(pos, end, value.alice_base_key);
--
GitLab