Skip to content

fix: reject corrupted attachment stream decrypts #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ap4sh wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix: reject corrupted attachment stream decrypts #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions src/attachments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ std::optional<size_t> decrypted_max_size(size_t encrypted_size) {
return sz;
}

static void throw_decryptor_failure() {
throw std::runtime_error{"Attachment decryption failed: invalid key or corrupted data"};
}

// We have to roll our own custom version of crypto_secretstream_xchacha20poly1305_init_push here
// because libsodium offers no way to provide the randomness it uses (it hard codes a call to
// randombytes_buf), and so this repeats its internal implementation but using our hashed data for
Expand Down Expand Up @@ -791,8 +795,8 @@ void decrypt(
out.write(reinterpret_cast<const char*>(data.data()), data.size());
}};

d.update(encrypted);
d.finalize();
if (!d.update(encrypted) || !d.finalize())
throw_decryptor_failure();
} catch (const std::exception& e) {
std::error_code ec;
std::filesystem::remove(filename, ec);
Expand Down Expand Up @@ -832,12 +836,15 @@ size_t decrypt(
}};

std::array<std::byte, 4096> chunk;
while (in.read(reinterpret_cast<char*>(chunk.data()), chunk.size()))
d.update(chunk);
if (in.gcount() > 0)
d.update(std::span{chunk}.first(in.gcount()));
while (in.read(reinterpret_cast<char*>(chunk.data()), chunk.size())) {
if (!d.update(chunk))
throw_decryptor_failure();
}
if (in.gcount() > 0 && !d.update(std::span{chunk}.first(in.gcount())))
throw_decryptor_failure();

d.finalize();
if (!d.finalize())
throw_decryptor_failure();

return decrypted - out.begin();
}
Expand Down Expand Up @@ -881,11 +888,14 @@ void decrypt(
}};

std::array<std::byte, 4096> chunk;
while (in.read(reinterpret_cast<char*>(chunk.data()), chunk.size()))
d.update(chunk);
if (in.gcount() > 0)
d.update(std::span{chunk}.first(in.gcount()));
d.finalize();
while (in.read(reinterpret_cast<char*>(chunk.data()), chunk.size())) {
if (!d.update(chunk))
throw_decryptor_failure();
}
if (in.gcount() > 0 && !d.update(std::span{chunk}.first(in.gcount())))
throw_decryptor_failure();
if (!d.finalize())
throw_decryptor_failure();
} catch (const std::exception& e) {
std::error_code ec;
std::filesystem::remove(file_out, ec);
Expand Down
99 changes: 99 additions & 0 deletions tests/test_attachment_encrypt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,24 @@ static std::vector<std::byte> slurp_file(const std::filesystem::path& filename)
return contents;
}

static void write_file(const std::filesystem::path& filename, std::span<const std::byte> contents) {
std::ofstream out;
out.exceptions(std::ios::failbit | std::ios::badbit);
out.open(filename, std::ios::binary | std::ios::trunc);
out.write(reinterpret_cast<const char*>(contents.data()), contents.size());
}

static void corrupt_last_byte(std::vector<std::byte>& data) {
REQUIRE(!data.empty());
data.back() ^= std::byte{0x01};
}

static void corrupt_first_full_chunk(std::vector<std::byte>& data) {
constexpr size_t first_chunk_offset = 1 + attachment::ENCRYPT_HEADER;
REQUIRE(data.size() > first_chunk_offset + attachment::ENCRYPTED_CHUNK_TOTAL);
data[first_chunk_offset] ^= std::byte{0x01};
}

TEST_CASE(
"Attachment encryption: plaintext buffer to encrypted file",
"[attachments][files][encrypt]") {
Expand Down Expand Up @@ -362,3 +380,84 @@ TEST_CASE(
CHECK(contents.size() == data.size());
CHECK(!!(contents == data));
}

TEST_CASE(
"Attachment streaming decryption rejects corrupted data", "[attachments][files][decrypt]") {

auto seed = "a123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"_hex_b;
auto data = make_data(1000);

auto [enc, key] = attachment::encrypt(seed, data, attachment::Domain::ATTACHMENT);
corrupt_last_byte(enc);

SECTION("encrypted buffer to plaintext file") {
temp_data_file out;

CHECK_THROWS_MATCHES(
attachment::decrypt(enc, key, out.path), std::runtime_error, bad_data_message);
CHECK_FALSE(std::filesystem::exists(out.path));
}

SECTION("encrypted file to plaintext buffer") {
temp_data_file encrypted_file;
write_file(encrypted_file.path, enc);

CHECK_THROWS_MATCHES(
attachment::decrypt(encrypted_file.path, key),
std::runtime_error,
bad_data_message);
}

SECTION("encrypted file to plaintext file") {
temp_data_file encrypted_file;
temp_data_file out;
write_file(encrypted_file.path, enc);

CHECK_THROWS_MATCHES(
attachment::decrypt(encrypted_file.path, key, out.path),
std::runtime_error,
bad_data_message);
CHECK_FALSE(std::filesystem::exists(out.path));
}
}

TEST_CASE(
"Attachment streaming decryption rejects corrupted full chunks",
"[attachments][files][decrypt]") {

auto seed = "b123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"_hex_b;
auto data = make_data(attachment::ENCRYPT_CHUNK_SIZE * 2);

auto [enc, key] = attachment::encrypt(seed, data, attachment::Domain::ATTACHMENT);
corrupt_first_full_chunk(enc);

SECTION("encrypted buffer to plaintext file") {
temp_data_file out;

CHECK_THROWS_MATCHES(
attachment::decrypt(enc, key, out.path), std::runtime_error, bad_data_message);
CHECK_FALSE(std::filesystem::exists(out.path));
}

SECTION("encrypted file to plaintext buffer") {
temp_data_file encrypted_file;
write_file(encrypted_file.path, enc);

CHECK_THROWS_MATCHES(
attachment::decrypt(encrypted_file.path, key),
std::runtime_error,
bad_data_message);
}

SECTION("encrypted file to plaintext file") {
temp_data_file encrypted_file;
temp_data_file out;
write_file(encrypted_file.path, enc);

CHECK_THROWS_MATCHES(
attachment::decrypt(encrypted_file.path, key, out.path),
std::runtime_error,
bad_data_message);
CHECK_FALSE(std::filesystem::exists(out.path));
}
}