[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1622937 -/subversion/trunk/subversion/libsvn_fs_fs/low_level.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Sat, 6 Sep 2014 23:54:32 +0200

The dirent is canonical test is not the right test for fspaths. Dirents are platform specific, while fspaths aren't. E.g. this breaks fspaths containing ':' on Windows.

I think there should be a valid fspath API for this use case, and if not we should add it.

Bert

-----Original Message-----
From: "stefan2_at_apache.org" <stefan2_at_apache.org>
Sent: ‎06/‎09/‎2014 23:16
To: "commits_at_subversion.apache.org" <commits_at_subversion.apache.org>
Subject: svn commit: r1622937 -/subversion/trunk/subversion/libsvn_fs_fs/low_level.c

Author: stefan2
Date: Sat Sep 6 21:15:40 2014
New Revision: 1622937

URL: http://svn.apache.org/r1622937
Log:
Harden FSFS's low-level path parsers against data corruption.

* subversion/libsvn_fs_fs/low_level.c
  (read_change
   svn_fs_fs__read_noderev): All paths must be canonical and start with '/'

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/low_level.c

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=1622937&r1=1622936&r2=1622937&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:15:40 2014
@@ -21,6 +21,7 @@
  */
 
 #include "svn_private_config.h"
+#include "svn_dirent_uri.h"
 #include "svn_hash.h"
 #include "svn_pools.h"
 #include "svn_sorts.h"
@@ -376,8 +377,12 @@ read_change(change_t **change_p,
                               _("Invalid mergeinfo-mod flag in rev-file"));
         }
     }
-
+
   /* Get the changed path. */
+ if (*last_str != '/' || !svn_dirent_is_canonical(last_str, scratch_pool))
+ return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
+ _("Invalid path in changes line"));
+
   change->path.len = strlen(last_str);
   change->path.data = apr_pstrdup(result_pool, last_str);
 
@@ -394,9 +399,10 @@ read_change(change_t **change_p,
       last_str = line->data;
       SVN_ERR(parse_revnum(&info->copyfrom_rev, (const char **)&last_str));
 
- if (! last_str)
+ if ( *last_str != '/'
+ || !svn_dirent_is_canonical(last_str, scratch_pool))
         return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
- _("Invalid changes line in rev-file"));
+ _("Invalid copy-from path in changes line"));
 
       info->copyfrom_path = apr_pstrdup(result_pool, last_str);
     }
@@ -869,6 +875,11 @@ svn_fs_fs__read_noderev(node_revision_t
     }
   else
     {
+ if (*value != '/' || !svn_dirent_is_canonical(value, scratch_pool))
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ _("Non-canonical cpath field in node-rev '%s'"),
+ noderev_id);
+
       noderev->created_path = apr_pstrdup(result_pool, value);
     }
 
@@ -888,7 +899,7 @@ svn_fs_fs__read_noderev(node_revision_t
     {
       SVN_ERR(parse_revnum(&noderev->copyroot_rev, (const char **)&value));
 
- if (*value == '\0')
+ if (*value != '/' || !svn_dirent_is_canonical(value, scratch_pool))
         return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
                                  _("Malformed copyroot line in node-rev '%s'"),
                                  noderev_id);
Received on 2014-09-06 23:56:09 CEST

This is an archived mail posted to the Subversion Dev mailing list.