You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/09/06 23:44:19 UTC

svn commit: r1622942 - in /subversion/trunk/subversion: include/svn_error_codes.h libsvn_fs_fs/cached_data.c libsvn_fs_fs/fs_fs.c libsvn_fs_fs/id.c libsvn_fs_fs/id.h libsvn_fs_fs/low_level.c libsvn_fs_fs/recovery.c

Author: stefan2
Date: Sat Sep  6 21:44:19 2014
New Revision: 1622942

URL: http://svn.apache.org/r1622942
Log:
Harden FSFS parsers against corrupted ID strings. 

Currently, we would simply return a NULL ID in case of invalid input.
That leads to NULL struct members in other places - which is a bad idea.
Make the ID parser follow the usual svn_error_t * return pattern.

* subversion/include/svn_error_codes.h
  (SVN_ERR_FS_MALFORMED_NODEREV_ID): Declare a new error code.

* subversion/libsvn_fs_fs/id.h
  (svn_fs_fs__id_copy): Change signature to return svn_error_t *.

* subversion/libsvn_fs_fs/id.c
  (svn_fs_fs__id_parse): Rename to ...
  (id_parse): ... this and make it static.
  (svn_fs_fs__id_parse): New, error generating wrapper.

* subversion/libsvn_fs_fs/cached_data.c
  (read_dir_entries): Update caller.

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__get_node_origin): Same.

* subversion/libsvn_fs_fs/low_level.c
  (read_change,
   svn_fs_fs__read_noderev): Same.

* subversion/libsvn_fs_fs/recovery.c
  (recover_find_max_ids): Same.

Modified:
    subversion/trunk/subversion/include/svn_error_codes.h
    subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_fs_fs/id.c
    subversion/trunk/subversion/libsvn_fs_fs/id.h
    subversion/trunk/subversion/libsvn_fs_fs/low_level.c
    subversion/trunk/subversion/libsvn_fs_fs/recovery.c

Modified: subversion/trunk/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_error_codes.h?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_error_codes.h (original)
+++ subversion/trunk/subversion/include/svn_error_codes.h Sat Sep  6 21:44:19 2014
@@ -856,6 +856,11 @@ SVN_ERROR_START
              SVN_ERR_FS_CATEGORY_START + 61,
              "Container capacity exceeded.")
 
+  /** @since New in 1.9. */
+  SVN_ERRDEF(SVN_ERR_FS_MALFORMED_NODEREV_ID,
+             SVN_ERR_FS_CATEGORY_START + 62,
+             "Malformed node revision ID string.")
+
   /* repos errors */
 
   SVN_ERRDEF(SVN_ERR_REPOS_LOCKED,

Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Sat Sep  6 21:44:19 2014
@@ -2393,7 +2393,7 @@ read_dir_entries(apr_array_header_t *ent
                            _("Directory entry corrupt in '%s'"),
                            svn_fs_fs__id_unparse(id, scratch_pool)->data);
 
-      dirent->id = svn_fs_fs__id_parse(str, result_pool);
+      SVN_ERR(svn_fs_fs__id_parse(&dirent->id, str, result_pool));
 
       /* In incremental mode, update the hash; otherwise, write to the
        * final array.  Be sure to use hash keys that survive this iteration.

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Sep  6 21:44:19 2014
@@ -1855,9 +1855,9 @@ svn_fs_fs__get_node_origin(const svn_fs_
         = apr_hash_get(node_origins, node_id_ptr, len);
 
       if (origin_id_str)
-        *origin_id = svn_fs_fs__id_parse(apr_pstrdup(pool,
-                                                     origin_id_str->data),
-                                         pool);
+        SVN_ERR(svn_fs_fs__id_parse(origin_id,
+                                    apr_pstrdup(pool, origin_id_str->data),
+                                    pool));
     }
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_fs/id.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/id.c?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/id.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/id.c Sat Sep  6 21:44:19 2014
@@ -494,10 +494,11 @@ svn_fs_fs__id_copy(const svn_fs_id_t *so
   return (svn_fs_id_t *)new_id;
 }
 
-
-svn_fs_id_t *
-svn_fs_fs__id_parse(char *data,
-                    apr_pool_t *pool)
+/* Return an ID resulting from parsing the string DATA, or NULL if DATA is
+   an invalid ID string. *DATA will be modified / invalidated by this call. */
+static svn_fs_id_t *
+id_parse(char *data,
+         apr_pool_t *pool)
 {
   fs_fs__id_t *id;
   char *str;
@@ -573,6 +574,21 @@ svn_fs_fs__id_parse(char *data,
   return (svn_fs_id_t *)id;
 }
 
+svn_error_t *
+svn_fs_fs__id_parse(const svn_fs_id_t **id_p,
+                    char *data,
+                    apr_pool_t *pool)
+{
+  svn_fs_id_t *id = id_parse(data, pool);
+  if (id == NULL)
+    return svn_error_createf(SVN_ERR_FS_MALFORMED_NODEREV_ID, NULL,
+                             "Malformed node revision ID string");
+
+  *id_p = id;
+
+  return SVN_NO_ERROR;
+}
+
 /* (de-)serialization support */
 
 /* Serialize an ID within the serialization CONTEXT.

Modified: subversion/trunk/subversion/libsvn_fs_fs/id.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/id.h?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/id.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/id.h Sat Sep  6 21:44:19 2014
@@ -149,10 +149,13 @@ svn_fs_id_t *svn_fs_fs__id_rev_create(co
 svn_fs_id_t *svn_fs_fs__id_copy(const svn_fs_id_t *id,
                                 apr_pool_t *pool);
 
-/* Return an ID resulting from parsing the string DATA, or NULL if DATA is
-   an invalid ID string. *DATA will be modified / invalidated by this call. */
-svn_fs_id_t *svn_fs_fs__id_parse(char *data,
-                                 apr_pool_t *pool);
+/* Return an ID in *ID_P resulting from parsing the string DATA, or an error
+   if DATA is an invalid ID string. *DATA will be modified / invalidated by
+   this call. */
+svn_error_t *
+svn_fs_fs__id_parse(const svn_fs_id_t **id_p,
+                    char *data,
+                    apr_pool_t *pool);
 
 
 /* (de-)serialization support*/

Modified: subversion/trunk/subversion/libsvn_fs_fs/low_level.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/low_level.c?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/low_level.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/low_level.c Sat Sep  6 21:44:19 2014
@@ -259,7 +259,7 @@ read_change(change_t **change_p,
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid changes line in rev-file"));
 
-  info->node_rev_id = svn_fs_fs__id_parse(str, result_pool);
+  SVN_ERR(svn_fs_fs__id_parse(&info->node_rev_id, str, result_pool));
   if (info->node_rev_id == NULL)
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid changes line in rev-file"));
@@ -824,7 +824,7 @@ svn_fs_fs__read_noderev(node_revision_t 
 
   SVN_ERR(svn_stream_close(stream));
 
-  noderev->id = svn_fs_fs__id_parse(value, result_pool);
+  SVN_ERR(svn_fs_fs__id_parse(&noderev->id, value, result_pool));
   noderev_id = value; /* for error messages later */
 
   /* Read the type. */
@@ -886,7 +886,8 @@ svn_fs_fs__read_noderev(node_revision_t 
   /* Get the predecessor ID. */
   value = svn_hash_gets(headers, HEADER_PRED);
   if (value)
-    noderev->predecessor_id = svn_fs_fs__id_parse(value, result_pool);
+    SVN_ERR(svn_fs_fs__id_parse(&noderev->predecessor_id, value,
+                                result_pool));
 
   /* Get the copyroot. */
   value = svn_hash_gets(headers, HEADER_COPYROOT);

Modified: subversion/trunk/subversion/libsvn_fs_fs/recovery.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/recovery.c?rev=1622942&r1=1622941&r2=1622942&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/recovery.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/recovery.c Sat Sep  6 21:44:19 2014
@@ -216,7 +216,7 @@ recover_find_max_ids(svn_fs_t *fs,
       char *str_val;
       char *str;
       svn_node_kind_t kind;
-      svn_fs_id_t *id;
+      const svn_fs_id_t *id;
       const svn_fs_fs__id_part_t *rev_item;
       apr_uint64_t node_id, copy_id;
       apr_off_t child_dir_offset;
@@ -246,7 +246,7 @@ recover_find_max_ids(svn_fs_t *fs,
         return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                                 _("Directory entry corrupt"));
 
-      id = svn_fs_fs__id_parse(str, iterpool);
+      SVN_ERR(svn_fs_fs__id_parse(&id, str, iterpool));
 
       rev_item = svn_fs_fs__id_rev_item(id);
       if (rev_item->revision != rev)