Skip to content
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.

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

Replaced char array in output_stream to std::string to avoid buffer overflow #54

Merged
merged 3 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 13 additions & 7 deletions libraries/native/native/eosio/crt.hpp
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
#pragma once
#include <setjmp.h>

#include <string>

namespace eosio { namespace cdt {
enum output_stream_kind {
std_out,
std_err,
none
};
struct output_stream {
char output[1024*2];
size_t index = 0;
std::string to_string()const { return std::string((const char*)output, index); }
const char* get()const { return output; }
void push(char c) { output[index++] = c; }
void clear() { index = 0; }
class output_stream {
static constexpr size_t initial_size = 1024 * 4;
std::string output;

public:
output_stream() { output.reserve(initial_size); }
const std::string& to_string() const { return output; }
const char* get() const { return output.c_str(); }
size_t index() const { return output.size(); }
void push(char c) { output.push_back(c); }
void clear() { output.clear(); }
};
}} //ns eosio::cdt

Expand Down
4 changes: 2 additions & 2 deletions libraries/native/native/eosio/tester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ template <size_t N, typename F, typename... Args>
inline bool expect_assert(bool check, const std::string& li, const char (&expected)[N], F&& func, Args... args) {
return expect_assert(check, li,
[&](const std::string& s) {
return std_err.index == N-1 &&
return std_err.index() == N-1 &&
memcmp(expected, s.c_str(), N-1) == 0; }, func, args...);
}

Expand All @@ -75,7 +75,7 @@ template <size_t N, typename F, typename... Args>
inline bool expect_print(bool check, const std::string& li, const char (&expected)[N], F&& func, Args... args) {
return expect_print(check, li,
[&](const std::string& s) {
return std_out.index == N-1 &&
return std_out.index() == N-1 &&
memcmp(expected, s.c_str(), N-1) == 0; }, func, args...);

}
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ endmacro()

add_unit_test( asset_tests )
add_unit_test( binary_extension_tests )
add_unit_test( crt_tests )
add_unit_test( crypto_tests )
add_unit_test( crypto_ext_tests )
add_unit_test( datastream_tests )
Expand Down
1 change: 1 addition & 0 deletions tests/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ endmacro()

add_cdt_unit_test(asset_tests)
add_cdt_unit_test(binary_extension_tests)
add_cdt_unit_test(crt_tests)
add_cdt_unit_test(crypto_tests)
add_cdt_unit_test(crypto_ext_tests)
add_cdt_unit_test(datastream_tests)
Expand Down
69 changes: 69 additions & 0 deletions tests/unit/crt_tests.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* @file
* @copyright defined in eosio.cdt/LICENSE.txt
*/

#include <cstdint>
#include <cstdio>
#include <string>

#include <eosio/tester.hpp>
#include <eosio/crt.hpp>

using eosio::cdt::output_stream;

EOSIO_TEST_BEGIN(output_stream_push)
std_err.clear();
const char* msg = "abc";
_prints(msg, eosio::cdt::output_stream_kind::std_err);
CHECK_EQUAL(std_err.to_string(), "abc");
CHECK_EQUAL(std_err.index(), 3);

std_err.clear();
const char* msg2 = "";
_prints(msg2, eosio::cdt::output_stream_kind::std_err);
CHECK_EQUAL(std_err.to_string(), "");
CHECK_EQUAL(std_err.index(), 0);

std_err.clear();
EOSIO_TEST_END

EOSIO_TEST_BEGIN(output_stream_push_overflow)
std_err.clear();
const auto initial_capacity = std_err.to_string().capacity();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it worth to check initial_capaticy as well. if it will be zero test should pass anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial capacity of string is unspecified so it can be anything. Do you want to add:

Suggested change
const auto initial_capacity = std_err.to_string().capacity();
const auto initial_capacity = std_err.to_string().capacity();
CHECK_EQUAL(initial_capacity >= 0, true);

But this will be always true, because it is unsigned_int

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I thought we can check here hardcoded 4kb size but that might be too much of details.
So this looks resolved now.

CHECK_EQUAL(std_err.index(), 0);

std::string large_msg('x', initial_capacity + 1);

_prints(large_msg.c_str(), eosio::cdt::output_stream_kind::std_err);
CHECK_EQUAL(std_err.to_string().capacity() > initial_capacity, true);
CHECK_EQUAL(std_err.index(), large_msg.size());

std_err.clear();
EOSIO_TEST_END
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add get() and push() tests. I see clear(), to_string() and 'index()' calls only

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


EOSIO_TEST_BEGIN(output_stream_get_and_push)
std_err.clear();
std::string example_msg("abcdef");

_prints(example_msg.c_str(), eosio::cdt::output_stream_kind::std_err);
CHECK_EQUAL(strcmp(std_err.get(), "abcdef") , 0);

std_err.push('g');
CHECK_EQUAL(strcmp(std_err.get(), "abcdefg") , 0);

std_err.clear();
EOSIO_TEST_END

int main(int argc, char* argv[]) {
bool verbose = false;
if( argc >= 2 && std::strcmp( argv[1], "-v" ) == 0 ) {
verbose = true;
}
silence_output(!verbose);

EOSIO_TEST(output_stream_push)
EOSIO_TEST(output_stream_push_overflow)
EOSIO_TEST(output_stream_get_and_push)
return has_failed();
}