added more crash-protection when reading corrupted collection files at startup

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()

arangod/V8Server/v8-vocbase.cpp

@@ -1946,9 +1946,9 @@ static v8::Handle<v8::Value> 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());
 }
 
 ////////////////////////////////////////////////////////////////////////////////

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);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

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

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
 ////////////////////////////////////////////////////////////////////////////////

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
 ////////////////////////////////////////////////////////////////////////////////