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

Throw on invalid file #3203

Merged
merged 2 commits into from
Jan 3, 2019
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# NEXT RELEASE

### Enhancements
* None.
* Instead of asserting, an `InvalidDatabase` exception is thrown when a realm file is opened
with an invalid top ref.

### Fixed
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
Expand Down
2 changes: 1 addition & 1 deletion src/external/pegtl
Submodule pegtl updated 39 files
+12 −1 .travis.yml
+2 −2 CMakeLists.txt
+1 −1 Makefile
+2 −1 README.md
+17 −0 doc/Changelog.md
+8 −3 doc/Inputs-and-Parsing.md
+0 −5 doc/Installing-and-Using.md
+2 −0 doc/README.md
+7 −2 doc/Rule-Reference.md
+5 −0 include/tao/pegtl/argv_input.hpp
+1 −0 include/tao/pegtl/ascii.hpp
+1 −0 include/tao/pegtl/buffer_input.hpp
+2 −17 include/tao/pegtl/contrib/changes.hpp
+61 −0 include/tao/pegtl/contrib/if_then.hpp
+176 −88 include/tao/pegtl/contrib/parse_tree.hpp
+3 −3 include/tao/pegtl/contrib/raw_string.hpp
+8 −6 include/tao/pegtl/contrib/unescape.hpp
+5 −0 include/tao/pegtl/cstream_input.hpp
+17 −4 include/tao/pegtl/file_input.hpp
+8 −0 include/tao/pegtl/input_error.hpp
+38 −0 include/tao/pegtl/internal/conditional.hpp
+2 −2 include/tao/pegtl/internal/file_mapper_posix.hpp
+211 −0 include/tao/pegtl/internal/file_mapper_win32.hpp
+5 −0 include/tao/pegtl/istream_input.hpp
+7 −0 include/tao/pegtl/memory_input.hpp
+15 −1 include/tao/pegtl/mmap_input.hpp
+30 −0 include/tao/pegtl/parse.hpp
+5 −0 include/tao/pegtl/read_input.hpp
+5 −0 include/tao/pegtl/string_input.hpp
+2 −2 include/tao/pegtl/version.hpp
+1 −0 src/example/pegtl/CMakeLists.txt
+222 −153 src/example/pegtl/abnf2pegtl.cpp
+18 −23 src/example/pegtl/parse_tree.cpp
+6 −2 src/test/pegtl/CMakeLists.txt
+42 −0 src/test/pegtl/ascii_forty_two.cpp
+31 −0 src/test/pegtl/contrib_if_then.cpp
+60 −0 src/test/pegtl/contrib_parse_tree.cpp
+1 −1 src/test/pegtl/file_mmap.cpp
+61 −0 src/test/pegtl/tester.cpp
66 changes: 37 additions & 29 deletions src/realm/group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,42 @@ void Group::remap_and_update_refs(ref_type new_top_ref, size_t new_file_size)
update_refs(new_top_ref, old_baseline);
}

void Group::validate_top_array(const Array& arr, const SlabAlloc& alloc)
{
size_t top_size = arr.size();
ref_type top_ref = arr.get_ref();

switch (top_size) {
// These are the valid sizes
case 3:
case 5:
case 7:
case 9:
case 10: {
ref_type table_names_ref = arr.get_as_ref_or_tagged(0).get_as_ref();
ref_type tables_ref = arr.get_as_ref_or_tagged(1).get_as_ref();
auto logical_file_size = arr.get_as_ref_or_tagged(2).get_as_int();

// Logical file size must never exceed actual file size.
// First two entries must be valid refs pointing inside the file
auto file_size = alloc.get_baseline();
if (logical_file_size > file_size || table_names_ref == 0 || table_names_ref > logical_file_size ||
(table_names_ref & 7) || tables_ref == 0 || tables_ref > logical_file_size || (tables_ref & 7)) {
std::string err = "Invalid top array (ref, [0], [1], [2]): " + util::to_string(top_ref) + ", " +
util::to_string(table_names_ref) + ", " + util::to_string(tables_ref) + ", " +
util::to_string(logical_file_size);
throw InvalidDatabase(err, "");
}
break;
}
default: {
std::string err =
"Invalid top array (ref: " + util::to_string(top_ref) + ", size: " + util::to_string(top_size) + ")";
throw InvalidDatabase(err, "");
break;
}
}
}

void Group::attach(ref_type top_ref, bool create_group_when_missing)
{
Expand All @@ -298,37 +334,9 @@ void Group::attach(ref_type top_ref, bool create_group_when_missing)

if (top_ref != 0) {
m_top.init_from_ref(top_ref);
size_t top_size = m_top.size();

switch (top_size) {
// These are the valid sizes
case 3:
case 5:
case 7:
case 9:
case 10: {
ref_type table_names_ref = m_top.get_as_ref_or_tagged(0).get_as_ref();
ref_type tables_ref = m_top.get_as_ref_or_tagged(1).get_as_ref();
auto logical_file_size = m_top.get_as_ref_or_tagged(2).get_as_int();

// Logical file size must never exceed actual file size.
REALM_ASSERT_RELEASE_EX(logical_file_size <= m_alloc.get_baseline(), top_ref, logical_file_size);
// First two entries must be valid refs pointing inside the file
REALM_ASSERT_RELEASE_EX(table_names_ref > 0 && table_names_ref < logical_file_size &&
(table_names_ref & 7) == 0,
top_ref, table_names_ref);
REALM_ASSERT_RELEASE_EX(tables_ref > 0 && tables_ref < logical_file_size && (tables_ref & 7) == 0,
top_ref, tables_ref);
break;
}
default:
REALM_ASSERT_RELEASE_EX(!"Invalid top array size", top_ref, top_size);
break;
}
validate_top_array(m_top, m_alloc);
m_table_names.init_from_parent();
m_tables.init_from_parent();
REALM_ASSERT_RELEASE_EX(m_table_names.size() == m_tables.size(), top_ref, m_table_names.size(),
m_tables.size());
}
else if (create_group_when_missing) {
create_empty_group(); // Throws
Expand Down
1 change: 1 addition & 0 deletions src/realm/group.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,7 @@ class Group : private Table::Parent {
void set_history_schema_version(int version);
void set_history_parent(Array& history_root) noexcept;
void prepare_history_parent(Array& history_root, int history_type, int history_schema_version);
static void validate_top_array(const Array& arr, const SlabAlloc& alloc);

friend class Table;
friend class GroupWriter;
Expand Down
19 changes: 16 additions & 3 deletions src/realm/group_shared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1031,10 +1031,24 @@ void SharedGroup::do_open(const std::string& path, bool no_create_file, bool is_
ref_type top_ref;
try {
top_ref = alloc.attach_file(path, cfg); // Throws
if (top_ref) {
Array top{alloc};
top.init_from_ref(top_ref);
Group::validate_top_array(top, alloc);
}
}
catch (SlabAlloc::Retry&) {
catch (const SlabAlloc::Retry&) {
continue;
}
catch (const InvalidDatabase& e) {
if (e.get_path().size() == 0) {
std::string msg = e.what();
throw InvalidDatabase(msg, path);
}
else {
throw e;
}
}
// If we fail in any way, we must detach the allocator. Failure to do so
// will retain memory mappings in the mmap cache shared between allocators.
// This would allow other SharedGroups to reuse the mappings even in
Expand Down Expand Up @@ -1118,8 +1132,7 @@ void SharedGroup::do_open(const std::string& path, bool no_create_file, bool is_
break;
}

REALM_ASSERT(stored_hist_schema_version >= 0 &&
stored_hist_schema_version <= openers_hist_schema_version);
REALM_ASSERT(stored_hist_schema_version >= 0);
if (stored_hist_schema_version > openers_hist_schema_version)
throw IncompatibleHistories("Unexpected future history schema version", path);
bool need_hist_schema_upgrade =
Expand Down
11 changes: 10 additions & 1 deletion src/realm/util/file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <stdexcept>
#include <string>
#include <streambuf>
#include <iostream>

#ifndef _WIN32
#include <dirent.h> // POSIX.1-2001
Expand Down Expand Up @@ -874,7 +875,6 @@ class File::Streambuf : public std::streambuf {
void flush();
};


/// Used for any I/O related exception. Note the derived exception
/// types that are used for various specific types of errors.
class File::AccessError : public std::runtime_error {
Expand All @@ -885,8 +885,17 @@ class File::AccessError : public std::runtime_error {
/// no associated file system path, or if the file system path is unknown.
std::string get_path() const;

const char* what() const noexcept
{
m_buffer = std::runtime_error::what();
if (m_path.size() > 0)
m_buffer += (std::string(" Path: ") + m_path);
return m_buffer.c_str();
}

Copy link
Contributor

@kspangsege kspangsege Jan 8, 2019

Choose a reason for hiding this comment

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

@jedelbo, are you aware that this construct makes the exception class not thread-compatible?

Modern C++ paradigm dictates these rules:

A necessary requirement for a type to be thread-compatible, is that all its const methods are thread-safe, meaning that two threads can safely call const methods on that type concurrently, as long as no other thread is calling a non-const method.

A class should be considered thread-compatible unless it is clear from the context that it is not, such as via documentation.

Roughly speaking, a type is thread-compatible if it is not thread-hostile. A type is thread-hostile if it does not offer the same intuitive level of thread-safety as is offered by fundamental types (integers) and almost all STL types (e.g., std::string).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was aware of that.

private:
std::string m_path;
mutable std::string m_buffer;
};


Expand Down