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

Fix limits #100

Merged
merged 3 commits into from Jan 8, 2019
Merged

Fix limits #100

Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -6,4 +6,9 @@
#ifndef FC_MAX_LOG_OBJECT_DEPTH
// how many levels of nested objects are displayed in log messages
#define FC_MAX_LOG_OBJECT_DEPTH 200
#endif
#endif

#ifndef FC_MAX_PREALLOC_SIZE
// how many elements will be reserve()d when deserializing vectors
#define FC_MAX_PREALLOC_SIZE (200UL)
This conversation was marked as resolved by abitmore

This comment has been minimized.

Copy link
@abitmore

abitmore Jan 6, 2019

Member

Better change the default value to 2^x to get better performance since *2 is used in loop.

#endif
@@ -26,7 +26,7 @@ namespace fc {
unsigned_int size; unpack( s, size, _max_depth );
value.clear();
FC_ASSERT( size.value*sizeof(T) < MAX_ARRAY_ALLOC_SIZE );
value.reserve(size.value);
value.reserve( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint32_t i = 0; i < size.value; ++i )
{
T tmp;
@@ -54,7 +54,7 @@ namespace fc {
unsigned_int size; unpack( s, size, _max_depth );
value.clear();
FC_ASSERT( size.value*(sizeof(K)+sizeof(V)) < MAX_ARRAY_ALLOC_SIZE );
value.reserve(size.value);
value.reserve( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint32_t i = 0; i < size.value; ++i )
{
std::pair<K,V> tmp;
@@ -86,11 +86,16 @@ namespace fc {
--_max_depth;
unsigned_int size;
unpack( s, size, _max_depth );
value.resize( size );
if( !std::is_fundamental<T>::value ) {
for( auto& item : value )
unpack( s, item, _max_depth );
value.resize( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint64_t i = 0; i < size; i++ )
{
if( i >= value.size() )
This conversation was marked as resolved by abitmore

This comment has been minimized.

Copy link
@abitmore

abitmore Jan 6, 2019

Member

Here can probably be optimized to not compare i with value.size() every time. E.G.

max_size = FC_MAX_PREALLOC_SIZE;
for( i = 0; i < size; )
{
   if( i > 0 && max_size < size )
      max_size *= 2;
   value.resize( min( size, max_size ) );
   while( i < value.size() )
   {
      unpack(i);
      ++i;
   }
}

I'm not sure if it worth the efforts though.

This comment has been minimized.

Copy link
@pmconrad

pmconrad Jan 7, 2019

Author

I don't think it's worth it. My guess would be that a typical implementation of vector::size will directly return a member variable.

value.resize( std::min( 2*value.size(), size.value ) );
This conversation was marked as resolved by abitmore

This comment has been minimized.

Copy link
@abitmore

abitmore Jan 6, 2019

Member

Perhaps change 2*value.size() to value.size() + FC_MAX_PREALLOC_SIZE to be more defensive? Which is better?

This comment has been minimized.

Copy link
@pmconrad

pmconrad Jan 7, 2019

Author

(Presumably) resize can cost O(n), so with linear growth you arrive at O(n^2) for large n, whereas exponential growth will result in O(n*log(n)).

This comment has been minimized.

Copy link
@pmconrad

pmconrad Jan 7, 2019

Author

Also note that the vector will only be resized to 2*n if the attacker has already sent n items.

unpack( s, value[i], _max_depth );
}
} else {
value.resize( size );
s.read( (char*)value.data(), value.size() );
}
}
@@ -105,31 +105,4 @@ namespace fc {
FC_ASSERT( r == d.size() );
}
}

namespace raw {
namespace bip = boost::interprocess;

template<typename Stream, typename T, typename... A>
inline void pack( Stream& s, const bip::vector<T,A...>& value, uint32_t _max_depth=FC_PACK_MAX_DEPTH ) {
FC_ASSERT( _max_depth > 0 );
--_max_depth;
pack( s, unsigned_int(value.size()), _max_depth );
auto itr = value.begin();
auto end = value.end();
while( itr != end ) {
fc::raw::pack( s, *itr, _max_depth );
++itr;
}
}
template<typename Stream, typename T, typename... A>
inline void unpack( Stream& s, bip::vector<T,A...>& value, uint32_t _max_depth=FC_PACK_MAX_DEPTH ) {
FC_ASSERT( _max_depth > 0 );
--_max_depth;
unsigned_int size;
unpack( s, size, _max_depth );
value.clear(); value.resize(size);
for( auto& item : value )
fc::raw::unpack( s, item, _max_depth );
}
}
}
@@ -429,7 +429,7 @@ namespace fc {
unsigned_int size; fc::raw::unpack( s, size, _max_depth );
value.clear();
FC_ASSERT( size.value*sizeof(T) < MAX_ARRAY_ALLOC_SIZE );
value.reserve(size.value);
value.reserve( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint32_t i = 0; i < size.value; ++i )
{
T tmp;
@@ -475,7 +475,7 @@ namespace fc {
unsigned_int size; fc::raw::unpack( s, size, _max_depth );
value.clear();
FC_ASSERT( size.value*(sizeof(K)+sizeof(V)) < MAX_ARRAY_ALLOC_SIZE );
value.reserve(size.value);
value.reserve( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint32_t i = 0; i < size.value; ++i )
{
std::pair<K,V> tmp;
@@ -530,12 +530,12 @@ namespace fc {
--_max_depth;
unsigned_int size; fc::raw::unpack( s, size, _max_depth );
FC_ASSERT( size.value*sizeof(T) < MAX_ARRAY_ALLOC_SIZE );
value.resize(size.value);
auto itr = value.begin();
auto end = value.end();
while( itr != end ) {
fc::raw::unpack( s, *itr, _max_depth );
++itr;
value.resize( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint64_t i = 0; i < size; i++ )
{
if( i >= value.size() )
value.resize( std::min( 2*value.size(), size.value ) );
unpack( s, value[i], _max_depth );
}
}

@@ -558,12 +558,12 @@ namespace fc {
--_max_depth;
unsigned_int size; fc::raw::unpack( s, size, _max_depth );
FC_ASSERT( size.value*sizeof(T) < MAX_ARRAY_ALLOC_SIZE );
value.resize(size.value);
auto itr = value.begin();
auto end = value.end();
while( itr != end ) {
fc::raw::unpack( s, *itr, _max_depth );
++itr;
value.resize( std::min( size.value, FC_MAX_PREALLOC_SIZE ) );
for( uint64_t i = 0; i < size; i++ )
{
if( i >= value.size() )
value.resize( std::min( 2*value.size(), size.value ) );
unpack( s, value[i], _max_depth );
}
}

@@ -143,9 +143,9 @@ namespace fc { namespace raw {
--_max_depth;
unsigned_int vs;
unpack( s, vs, _max_depth );

FC_ASSERT( vs.value*sizeof(variant_object::entry) < MAX_ARRAY_ALLOC_SIZE );
mutable_variant_object mvo;
mvo.reserve(vs.value);
mvo.reserve( std::min( vs.value, FC_MAX_PREALLOC_SIZE ) );
for( uint32_t i = 0; i < vs.value; ++i )
{
fc::string key;
@@ -102,4 +102,24 @@ BOOST_AUTO_TEST_CASE( nested_objects_test )

} FC_CAPTURE_LOG_AND_RETHROW ( (0) ) }

BOOST_AUTO_TEST_CASE( unpack_recursion_test )
{
try
{
std::stringstream ss;
int recursion_level = 100000;
uint64_t allocation_per_level = 500000;

for ( int i = 0; i < recursion_level; i++ )
{
fc::raw::pack( ss, fc::unsigned_int( allocation_per_level ) );
fc::raw::pack( ss, static_cast< uint8_t >( fc::variant::array_type ) );
}

std::vector< fc::variant > v;
BOOST_REQUIRE_THROW( fc::raw::unpack( ss, v ), fc::assert_exception );
}
FC_LOG_AND_RETHROW();
}

BOOST_AUTO_TEST_SUITE_END()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.