Commit 71bee30a authored by Denis Kasak's avatar Denis Kasak
Browse files

Fail when an unpickle succeeds but has extra junk data at the end.

Also adds tests to ensure this is working.
parent c85827ce
Pipeline #7853 passed with stages
in 2 minutes and 22 seconds
......@@ -271,9 +271,14 @@ size_t olm_unpickle_inbound_group_session(
} else {
pos = _olm_unpickle_bool(pos, end, &(session->signing_key_verified));
}
FAIL_ON_CORRUPTED_PICKLE(pos, session);
if (pos != end) {
/* Input was longer than expected. */
session->last_error = OLM_CORRUPTED_PICKLE;
return (size_t)-1;
}
return pickled_length;
}
......
......@@ -282,20 +282,31 @@ size_t olm_unpickle_account(
void * pickled, size_t pickled_length
) {
olm::Account & object = *from_c(account);
std::uint8_t * const pos = from_c(pickled);
std::uint8_t * input = from_c(pickled);
std::size_t raw_length = _olm_enc_input(
from_c(key), key_length, pos, pickled_length, &object.last_error
from_c(key), key_length, input, pickled_length, &object.last_error
);
if (raw_length == std::size_t(-1)) {
return std::size_t(-1);
}
std::uint8_t * const end = pos + raw_length;
if (!unpickle(pos, end, object)) {
std::uint8_t const * pos = input;
std::uint8_t const * end = pos + raw_length;
pos = unpickle(pos, end, object);
if (!pos) {
/* Input was corrupted. */
if (object.last_error == OlmErrorCode::OLM_SUCCESS) {
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
}
return std::size_t(-1);
} else if (pos != end) {
/* Input was longer than expected. */
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
return std::size_t(-1);
}
return pickled_length;
}
......@@ -306,21 +317,31 @@ size_t olm_unpickle_session(
void * pickled, size_t pickled_length
) {
olm::Session & object = *from_c(session);
std::uint8_t * const pos = from_c(pickled);
std::uint8_t * input = from_c(pickled);
std::size_t raw_length = _olm_enc_input(
from_c(key), key_length, pos, pickled_length, &object.last_error
from_c(key), key_length, input, pickled_length, &object.last_error
);
if (raw_length == std::size_t(-1)) {
return std::size_t(-1);
}
std::uint8_t * const end = pos + raw_length;
if (!unpickle(pos, end, object)) {
std::uint8_t const * pos = input;
std::uint8_t const * end = pos + raw_length;
pos = unpickle(pos, end, object);
if (!pos) {
/* Input was corrupted. */
if (object.last_error == OlmErrorCode::OLM_SUCCESS) {
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
}
return std::size_t(-1);
} else if (pos != end) {
/* Input was longer than expected. */
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
return std::size_t(-1);
}
return pickled_length;
}
......
......@@ -144,6 +144,12 @@ size_t olm_unpickle_outbound_group_session(
pos = _olm_unpickle_ed25519_key_pair(pos, end, &(session->signing_key));
FAIL_ON_CORRUPTED_PICKLE(pos, session);
if (pos != end) {
/* Input was longer than expected. */
session->last_error = OLM_CORRUPTED_PICKLE;
return (size_t)-1;
}
return pickled_length;
}
......
......@@ -326,22 +326,32 @@ size_t olm_unpickle_pk_decryption(
object.last_error = OlmErrorCode::OLM_OUTPUT_BUFFER_TOO_SMALL;
return std::size_t(-1);
}
std::uint8_t * const pos = reinterpret_cast<std::uint8_t *>(pickled);
std::uint8_t * const input = reinterpret_cast<std::uint8_t *>(pickled);
std::size_t raw_length = _olm_enc_input(
reinterpret_cast<std::uint8_t const *>(key), key_length,
pos, pickled_length, &object.last_error
input, pickled_length, &object.last_error
);
if (raw_length == std::size_t(-1)) {
return std::size_t(-1);
}
std::uint8_t * const end = pos + raw_length;
if (!unpickle(pos, end, object)) {
std::uint8_t const * pos = input;
std::uint8_t const * end = pos + raw_length;
pos = unpickle(pos, end, object);
if (!pos) {
/* Input was corrupted. */
if (object.last_error == OlmErrorCode::OLM_SUCCESS) {
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
}
return std::size_t(-1);
} else if (pos != end) {
/* Input was longer than expected. */
object.last_error = OlmErrorCode::OLM_CORRUPTED_PICKLE;
return std::size_t(-1);
}
if (pubkey != NULL) {
olm::encode_base64(
(const uint8_t *)object.key_pair.public_key.public_key,
......@@ -349,6 +359,7 @@ size_t olm_unpickle_pk_decryption(
(uint8_t *)pubkey
);
}
return pickled_length;
}
......
/* Copyright 2021 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <string.h>
#include "olm/pickle_encoding.h"
size_t add_junk_suffix_to_pickle(void const * key, size_t key_length,
void * pickled, size_t pickled_length, size_t junk_length)
{
size_t raw_length = _olm_enc_input((uint8_t const *) key, key_length,
(uint8_t *) pickled, pickled_length, nullptr);
/* Insert junk. */
size_t new_length = raw_length + junk_length;
uint8_t * pos = (uint8_t *) pickled + raw_length;
while (junk_length--) {
*pos++ = 255;
}
void * dest = _olm_enc_output_pos((uint8_t *) pickled, new_length);
memmove(dest, pickled, new_length);
return _olm_enc_output(
(uint8_t const *) key, key_length, (uint8_t *) pickled, new_length);
}
......@@ -15,6 +15,7 @@
#include "olm/inbound_group_session.h"
#include "olm/outbound_group_session.h"
#include "unittest.hh"
#include "utils.hh"
#include <vector>
......@@ -50,8 +51,31 @@ int main() {
assert_equals(pickle_length, res);
assert_equals(pickle1.data(), pickle2.data(), pickle_length);
}
/* Deliberately corrupt the pickled session by supplying a junk suffix and
* ensure this is caught as an error. */
const size_t junk_length = 1;
std::vector<std::uint8_t> junk_pickle(pickle_length + junk_length);
olm_pickle_outbound_group_session(
session, "secret_key", 10, junk_pickle.data(), pickle_length
);
const size_t junk_pickle_length = add_junk_suffix_to_pickle(
"secret_key", 10,
junk_pickle.data(),
pickle_length,
junk_length);
assert_equals(std::size_t(-1),
olm_unpickle_outbound_group_session(
session,
"secret_key", 10,
junk_pickle.data(), junk_pickle_length
));
assert_equals(OLM_CORRUPTED_PICKLE,
olm_outbound_group_session_last_error_code(session));
}
{
TestCase test_case("Pickle inbound group session");
......@@ -82,6 +106,30 @@ int main() {
);
assert_equals(pickle1.data(), pickle2.data(), pickle_length);
/* Deliberately corrupt the pickled session by supplying a junk suffix and
* ensure this is caught as an error. */
const size_t junk_length = 1;
std::vector<std::uint8_t> junk_pickle(pickle_length + junk_length);
olm_pickle_inbound_group_session(
session, "secret_key", 10, junk_pickle.data(), pickle_length
);
const size_t junk_pickle_length = add_junk_suffix_to_pickle(
"secret_key", 10,
junk_pickle.data(),
pickle_length,
junk_length);
assert_equals(std::size_t(-1),
olm_unpickle_inbound_group_session(
session,
"secret_key", 10,
junk_pickle.data(), junk_pickle_length
));
assert_equals(OLM_CORRUPTED_PICKLE,
olm_inbound_group_session_last_error_code(session));
}
{
......
#include "olm/olm.h"
#include "unittest.hh"
#include "utils.hh"
#include <cstddef>
#include <cstdint>
......@@ -65,6 +67,23 @@ res = ::olm_pickle_account(account2, "secret_key", 10, pickle2.data(), pickle_le
assert_equals(pickle_length, res);
assert_equals(pickle1.data(), pickle2.data(), pickle_length);
/* Deliberately corrupt the pickled account by supplying a junk suffix and
* ensure this is caught as an error. */
const size_t junk_length = 1;
std::vector<std::uint8_t> junk_pickle(pickle_length + junk_length);
res = ::olm_pickle_account(
account, "secret_key", 10, junk_pickle.data(), pickle_length);
assert_equals(pickle_length, res);
const size_t junk_pickle_length = add_junk_suffix_to_pickle(
"secret_key", 10, junk_pickle.data(), pickle_length, junk_length);
assert_equals(std::size_t(-1),
::olm_unpickle_account(account, "secret_key", 10,
junk_pickle.data(), junk_pickle_length));
assert_equals(OLM_CORRUPTED_PICKLE, olm_account_last_error_code(account));
}
......@@ -139,6 +158,23 @@ res = ::olm_pickle_session(session2, "secret_key", 10, pickle2.data(), pickle_le
assert_equals(pickle_length, res);
assert_equals(pickle1.data(), pickle2.data(), pickle_length);
/* Deliberately corrupt the pickled session by supplying a junk suffix and
* ensure this is caught as an error. */
const size_t junk_length = 1;
std::vector<std::uint8_t> junk_pickle(pickle_length + junk_length);
res = ::olm_pickle_session(
session, "secret_key", 10, junk_pickle.data(), pickle_length);
assert_equals(pickle_length, res);
const size_t junk_pickle_length = add_junk_suffix_to_pickle(
"secret_key", 10, junk_pickle.data(), pickle_length, junk_length);
assert_equals(std::size_t(-1),
::olm_unpickle_session(session, "secret_key", 10,
junk_pickle.data(), junk_pickle_length));
assert_equals(OLM_CORRUPTED_PICKLE, olm_session_last_error_code(session));
}
{ /** Loopback test */
......
#include "olm/pk.h"
#include "olm/crypto.h"
#include "olm/olm.h"
#include "olm/pk.h"
#include "unittest.hh"
#include "utils.hh"
#include <iostream>
#include <vector>
......@@ -141,6 +142,34 @@ olm_unpickle_pk_decryption(
assert_equals(alice_public, pubkey.data(), olm_pk_key_length());
/* Deliberately corrupt the pickled session by supplying a junk suffix and
* ensure this is caught as an error. */
const size_t junk_length = 1;
std::size_t pickle_length = olm_pickle_pk_decryption_length(decryption);
std::vector<std::uint8_t> junk_pickle(pickle_length + junk_length);
olm_pickle_pk_decryption(
decryption,
PICKLE_KEY, strlen((char *)PICKLE_KEY),
junk_pickle.data(), pickle_length
);
const size_t junk_pickle_length = add_junk_suffix_to_pickle(
PICKLE_KEY, strlen((char *)PICKLE_KEY),
junk_pickle.data(),
pickle_length,
junk_length);
assert_equals(std::size_t(-1),
olm_unpickle_pk_decryption(
decryption,
PICKLE_KEY, strlen((char *)PICKLE_KEY),
junk_pickle.data(), junk_pickle_length,
pubkey.data(), pubkey.size()
));
assert_equals(OLM_CORRUPTED_PICKLE, olm_pk_decryption_last_error_code(decryption));
/***/
char *ciphertext = strdup("ntk49j/KozVFtSqJXhCejg");
const char *mac = "zpzU6BkZcNI";
const char *ephemeral_key = "3p7bfXt9wbTTW2HC7OQ1Nz+DQ8hbeGdNrfx+FG+IK08";
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment