diff --git a/CHANGELOG b/CHANGELOG index 303e6a2d45..1a6699baa3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ v1.2.alpha (XXXX-XX-XX) ----------------------- +* added more crash-protection when reading corrupted collections at startup + * added documentation for AQL function CONTAINS() * added AQL function LIKE() diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index 4dc6c48194..c06049c288 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -1946,9 +1946,9 @@ static v8::Handle JS_ReloadAuth (v8::Arguments const& argv) { "usage: RELOAD_AUTH()"))); } - TRI_ReloadAuthInfo(vocbase); + bool result = TRI_ReloadAuthInfo(vocbase); - return scope.Close(v8::True()); + return scope.Close(result ? v8::True() : v8::False()); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/VocBase/auth.c b/arangod/VocBase/auth.c index 9f6ffde1db..d1a69a333b 100644 --- a/arangod/VocBase/auth.c +++ b/arangod/VocBase/auth.c @@ -234,7 +234,7 @@ static TRI_vocbase_auth_t* ConvertAuthInfo (TRI_vocbase_t* vocbase, /// @brief loads the authentication info //////////////////////////////////////////////////////////////////////////////// -void TRI_LoadAuthInfo (TRI_vocbase_t* vocbase) { +bool TRI_LoadAuthInfo (TRI_vocbase_t* vocbase) { TRI_vocbase_col_t* collection; TRI_primary_collection_t* primary; void** beg; @@ -247,7 +247,7 @@ void TRI_LoadAuthInfo (TRI_vocbase_t* vocbase) { if (collection == NULL) { LOG_INFO("collection '_users' does not exist, no authentication available"); - return; + return false; } TRI_UseCollectionVocBase(vocbase, collection); @@ -256,13 +256,13 @@ void TRI_LoadAuthInfo (TRI_vocbase_t* vocbase) { if (primary == NULL) { LOG_FATAL("collection '_users' cannot be loaded"); - return; + return false; } if (! TRI_IS_DOCUMENT_COLLECTION(primary->base._info._type)) { TRI_ReleaseCollectionVocBase(vocbase, collection); LOG_FATAL("collection '_users' has an unknown collection type"); - return; + return false; } TRI_WriteLockReadWriteLock(&vocbase->_authInfoLock); @@ -313,6 +313,8 @@ void TRI_LoadAuthInfo (TRI_vocbase_t* vocbase) { TRI_WriteUnlockReadWriteLock(&vocbase->_authInfoLock); TRI_ReleaseCollectionVocBase(vocbase, collection); + + return true; } //////////////////////////////////////////////////////////////////////////////// @@ -325,11 +327,13 @@ void TRI_DefaultAuthInfo (TRI_vocbase_t* vocbase) { //////////////////////////////////////////////////////////////////////////////// /// @brief reload the authentication info +/// this must be executed after the underlying _users collection is modified //////////////////////////////////////////////////////////////////////////////// -void TRI_ReloadAuthInfo (TRI_vocbase_t* vocbase) { +bool TRI_ReloadAuthInfo (TRI_vocbase_t* vocbase) { TRI_DestroyAuthInfo(vocbase); - TRI_LoadAuthInfo(vocbase); + + return TRI_LoadAuthInfo(vocbase); } //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/VocBase/auth.h b/arangod/VocBase/auth.h index 5f2e5ab760..3da6f17c25 100644 --- a/arangod/VocBase/auth.h +++ b/arangod/VocBase/auth.h @@ -77,7 +77,7 @@ TRI_vocbase_auth_t; /// @brief loads the authentication info //////////////////////////////////////////////////////////////////////////////// -void TRI_LoadAuthInfo (struct TRI_vocbase_s*); +bool TRI_LoadAuthInfo (struct TRI_vocbase_s*); //////////////////////////////////////////////////////////////////////////////// /// @brief sets the default authentication info @@ -87,10 +87,10 @@ void TRI_DefaultAuthInfo (struct TRI_vocbase_s*); //////////////////////////////////////////////////////////////////////////////// /// @brief reload the authentication info -/// this must be executed when the underlying _users collection is modified +/// this must be executed after the underlying _users collection is modified //////////////////////////////////////////////////////////////////////////////// -void TRI_ReloadAuthInfo (struct TRI_vocbase_s*); +bool TRI_ReloadAuthInfo (struct TRI_vocbase_s*); //////////////////////////////////////////////////////////////////////////////// /// @brief destroys the default authentication info diff --git a/arangod/VocBase/datafile.c b/arangod/VocBase/datafile.c index 9b25aa0268..d1407efe4d 100644 --- a/arangod/VocBase/datafile.c +++ b/arangod/VocBase/datafile.c @@ -538,6 +538,18 @@ static bool CheckDatafile (TRI_datafile_t* datafile) { return false; } + // the following sanity check offers some, but not 100% crash-protection when reading + // totally corrupted datafiles + if (! TRI_IsValidMarkerDatafile(marker)) { + datafile->_lastError = TRI_set_errno(TRI_ERROR_ARANGO_CORRUPTED_DATAFILE); + datafile->_currentSize = currentSize; + datafile->_next = datafile->_data + datafile->_currentSize; + datafile->_state = TRI_DF_STATE_OPEN_ERROR; + + LOG_WARNING("marker in datafile '%s' is corrupt", datafile->getName(datafile)); + return false; + } + ok = TRI_CheckCrcMarkerDatafile(marker); if (! ok) { @@ -628,7 +640,7 @@ static TRI_datafile_t* OpenDatafile (char const* filename, bool ignoreErrors) { TRI_set_errno(TRI_ERROR_ARANGO_CORRUPTED_DATAFILE); TRI_CLOSE(fd); - LOG_ERROR("datafile '%s' is corrupted, size is only %u", filename, (unsigned int) size); + LOG_ERROR("datafile '%s' is corrupt, size is only %u", filename, (unsigned int) size); return NULL; } @@ -973,6 +985,38 @@ void TRI_FreeDatafile (TRI_datafile_t* datafile) { /// @{ //////////////////////////////////////////////////////////////////////////////// +//////////////////////////////////////////////////////////////////////////////// +/// @brief checks whether a marker is valid +//////////////////////////////////////////////////////////////////////////////// + +bool TRI_IsValidMarkerDatafile (TRI_df_marker_t* const marker) { + TRI_df_marker_type_t type; + + if (marker == 0) { + return false; + } + + // check marker type + type = marker->_type; + if (type <= (TRI_df_marker_type_t) TRI_MARKER_MIN) { + // marker type is less than minimum allowed type value + return false; + } + + if (type >= (TRI_df_marker_type_t) TRI_MARKER_MAX) { + // marker type is greater than maximum allowed type value + return false; + } + + if (marker->_size >= (TRI_voc_size_t) (256 * 1024 * 1024)) { + // a single marker bigger than 256 MB seems unreasonable + // note: this is an arbitrary limit + return false; + } + + return true; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief checks a CRC of a marker //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/VocBase/datafile.h b/arangod/VocBase/datafile.h index 9e33609446..4b1d5e451b 100644 --- a/arangod/VocBase/datafile.h +++ b/arangod/VocBase/datafile.h @@ -146,6 +146,9 @@ TRI_df_state_e; //////////////////////////////////////////////////////////////////////////////// typedef enum { + TRI_MARKER_MIN = 999, // not a real marker type, + // but used for bounds checking + TRI_DF_MARKER_HEADER = 1000, TRI_DF_MARKER_FOOTER = 1001, TRI_DF_MARKER_SKIP = 1002, // currently unused @@ -164,7 +167,12 @@ typedef enum { TRI_DOC_MARKER_KEY_DOCUMENT = 3007, // new marker with key values TRI_DOC_MARKER_KEY_EDGE = 3008, // new marker with key values - TRI_DOC_MARKER_KEY_DELETION = 3009 // new marker with key values + TRI_DOC_MARKER_KEY_DELETION = 3009, // new marker with key values + + + TRI_MARKER_MAX // again, this is not a real + // marker, but we use it for + // bounds checking } TRI_df_marker_type_e; @@ -484,6 +492,12 @@ void TRI_FreeDatafile (TRI_datafile_t*); #define TRI_DF_ALIGN_BLOCK(a) ((((a) + TRI_DF_BLOCK_ALIGNMENT - 1) / TRI_DF_BLOCK_ALIGNMENT) * TRI_DF_BLOCK_ALIGNMENT) +//////////////////////////////////////////////////////////////////////////////// +/// @brief checks whether a marker is valid +//////////////////////////////////////////////////////////////////////////////// + +bool TRI_IsValidMarkerDatafile (TRI_df_marker_t* const marker); + //////////////////////////////////////////////////////////////////////////////// /// @brief checks a CRC of a marker ////////////////////////////////////////////////////////////////////////////////