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

[PATCH] fsfs7 index offset validation

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 17 May 2015 23:16:17 +0000

Stefan,

How about the following patch to sanity check the rev file footer?

[[[
fsfs7: Validate index offsets in rev file footer.

* subversion/libsvn_fs_fs/low_level.c
  (svn_fs_fs__parse_footer): Take FILESIZE argument, use it for validation.

* subversion/libsvn_fs_fs/low_level.h
  (svn_fs_fs__parse_footer): Take FILESIZE argument.

* subversion/libsvn_fs_fs/rev_file.c
  (svn_fs_fs__auto_read_footer): Pass FILESIZE.
]]]

[[[
Index: subversion/libsvn_fs_fs/low_level.c
===================================================================
--- subversion/libsvn_fs_fs/low_level.c (revision 1679906)
+++ subversion/libsvn_fs_fs/low_level.c (working copy)
@@ -196,9 +196,10 @@ svn_fs_fs__parse_footer(apr_off_t *l2p_offset,
                         svn_checksum_t **p2l_checksum,
                         svn_stringbuf_t *footer,
                         svn_revnum_t rev,
+ svn_filesize_t filesize,
                         apr_pool_t *result_pool)
 {
- apr_int64_t val;
+ apr_uint64_t val;
   char *last_str = footer->data;
 
   /* Get the L2P offset. */
@@ -207,7 +208,8 @@ svn_fs_fs__parse_footer(apr_off_t *l2p_offset,
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid revision footer"));
 
- SVN_ERR(svn_cstring_atoi64(&val, str));
+ SVN_ERR_W(svn_cstring_strtoui64(&val, str, 0, filesize-1, 10),
+ "Invalid L2P offset in revision footer");
   *l2p_offset = (apr_off_t)val;
 
   /* Get the L2P checksum. */
@@ -225,7 +227,9 @@ svn_fs_fs__parse_footer(apr_off_t *l2p_offset,
     return svn_error_create(SVN_ERR_FS_CORRUPT, NULL,
                             _("Invalid revision footer"));
 
- SVN_ERR(svn_cstring_atoi64(&val, str));
+ SVN_ERR_W(svn_cstring_strtoui64(&val, str, 0, filesize-1, 10),
+ "Invalid P2L offset in revision footer");
+
   *p2l_offset = (apr_off_t)val;
 
   /* Get the P2L checksum. */
Index: subversion/libsvn_fs_fs/low_level.h
===================================================================
--- subversion/libsvn_fs_fs/low_level.h (revision 1679906)
+++ subversion/libsvn_fs_fs/low_level.h (working copy)
@@ -67,6 +67,8 @@ svn_fs_fs__unparse_revision_trailer(apr_off_t root
  * *P2L_OFFSET, respectively. Also, return the expected checksums in
  * in *L2P_CHECKSUM and *P2L_CHECKSUM.
  *
+ * FILESIZE is used for validation.
+ *
  * Note that REV is only used to construct nicer error objects that
  * mention this revision. Allocate the checksums in RESULT_POOL.
  */
@@ -77,6 +79,7 @@ svn_fs_fs__parse_footer(apr_off_t *l2p_offset,
                         svn_checksum_t **p2l_checksum,
                         svn_stringbuf_t *footer,
                         svn_revnum_t rev,
+ svn_filesize_t filesize,
                         apr_pool_t *result_pool);
 
 /* Given the offset of the L2P index data in L2P_OFFSET, the content
Index: subversion/libsvn_fs_fs/rev_file.c
===================================================================
--- subversion/libsvn_fs_fs/rev_file.c (revision 1679906)
+++ subversion/libsvn_fs_fs/rev_file.c (working copy)
@@ -259,7 +259,9 @@ svn_fs_fs__auto_read_footer(svn_fs_fs__revision_fi
       SVN_ERR(svn_fs_fs__parse_footer(&file->l2p_offset, &file->l2p_checksum,
                                       &file->p2l_offset, &file->p2l_checksum,
                                       footer, file->start_revision,
+ filesize,
                                       file->pool));
+
       file->footer_offset = filesize - footer_length - 1;
     }
 
]]]

The error looks like this:

    subversion/libsvn_fs_fs/rev_file.c:263,
    subversion/libsvn_fs_fs/low_level.c:212: (apr_err=SVN_ERR_INCORRECT_PARAMS)
    svn: E200004: Invalid L2P offset in revision footer
    subversion/libsvn_subr/string.c:986: (apr_err=SVN_ERR_INCORRECT_PARAMS)
    svn: E200004: Number '888' is out of range '[0, 546]'

Daniel

P.S. In an earlier version of the patch I had some trouble getting an
apr_off_t to be printed in hex. (The working answer: use the %qx
formatter and cast to unsigned long long.) Constants such as
APR_OFF_T_FMT shouldn't include the "d" output modifier, only the "ll"
input modifier...
Received on 2015-05-18 01:18:39 CEST

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