Bugzilla – Attachment 23675 Details for
Bug 60610
VUL-0: CVE-2004-0749: Subversion/mod_authz_svn
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
IDP Log In
|
Forgot Password
[patch]
revprop.patch
revprop.patch (text/plain), 19.89 KB, created by
Marcus Meissner
on 2004-09-20 15:55:46 UTC
(
hide
)
Description:
revprop.patch
Filename:
MIME Type:
Creator:
Marcus Meissner
Created:
2004-09-20 15:55:46 UTC
Size:
19.89 KB
patch
obsolete
>Security fix for 1.0: authorize fetching & setting revprops. > >Implement new svn_repos__fs_revision_prop() and >svn_repos__fs_revision_proplist() wrappers, both of which take a >read_authz callback. Also define a new >svn_repos__fs_change_rev_prop2() that takes a read_authz callback. >Have mod_dav_svn call these new functions privately. > >If a revision's changed-paths are entirely unreadable: > - return NULL value when fetching any revprop. > - disallow all revprop writes (throw an APR_EGENERAL error) > >If a revision has a mixture of readable/unreadable changed-paths: > - return 'svn:author' and 'svn:date', but NULL values for anything else. > - disallow all property writes (throw an APR_EGENERAL error) > >(Note: this patch assumes you've applied the 'common' patch first.) > >* libsvn_repos/repos.h > (svn_repos__fs_revision_prop): new wrapper for svn_fs_revision_prop. > (svn_repos__fs_revision_proplist): new wrapper for svn_fs_revision_proplist. > (svn_repos__fs_change_rev_prop2): new func, takes authz callback. > >* libsvn_repos/fs-wrap.c > (enum readability_level, get_readability): new helper enum and func. > (svn_repos__fs_revision_prop): new func. > (svn_repos__fs_revision_proplist): new func. > (svn_repos__fs_change_rev_prop2): new func. > (svn_repos_fs_change_rev_prop): implement as dumb wrapper. > >* mod_dav_svn/deadprops.c > (dav_db): grow new fields to hold authz_func and baton. > (dav_svn_db_open): store dav_svn_authz_read() and baton in dav_db context. > (get_value, dav_svn_db_exists, dav_svn_db_first_name, save_value, > dav_svn_db_remove): call new repos wrapper with authz_func. > >* mod_dav_svn/liveprops.c > (dav_svn_insert_prop, dav_svn_get_last_modified_time): call new > repos wrapper with dav_svn_authz_read(). > > >Index: subversion/mod_dav_svn/deadprops.c >=================================================================== >--- subversion/mod_dav_svn/deadprops.c (revision 10978) >+++ subversion/mod_dav_svn/deadprops.c (working copy) >@@ -30,6 +30,8 @@ > #include "svn_dav.h" > #include "svn_base64.h" > >+/* Private header included just for 1.0.7 revprops security fix. */ >+#include "../libsvn_repos/repos.h" > > struct dav_db { > const dav_resource *resource; >@@ -41,6 +43,10 @@ > > /* used for constructing repos-local names for properties */ > svn_stringbuf_t *work; >+ >+ /* passed to svn_repos_ funcs that fetch revprops. */ >+ svn_repos_authz_func_t authz_read_func; >+ void *authz_read_baton; > }; > > struct dav_deadprop_rollback { >@@ -102,9 +108,12 @@ > serr = svn_fs_txn_prop(pvalue, db->resource->info->root.txn, > propname, db->p); > else >- serr = svn_fs_revision_prop(pvalue, db->resource->info->repos->fs, >- db->resource->info->root.rev, >- propname, db->p); >+ serr = svn_repos__fs_revision_prop >+ (pvalue, >+ db->resource->info-> repos->repos, >+ db->resource->info->root.rev, propname, >+ db->authz_read_func, >+ db->authz_read_baton, db->p); > else > serr = svn_fs_node_prop(pvalue, db->resource->info->root.root, > get_repos_path(db->resource->info), >@@ -148,10 +157,14 @@ > not a working resource! But this is how we currently > (hackily) allow the svn client to change unversioned rev > props. See issue #916. */ >- serr = svn_repos_fs_change_rev_prop(db->resource->info->repos->repos, >- db->resource->info->root.rev, >- db->resource->info->repos->username, >- propname, value, db->resource->pool); >+ serr = svn_repos__fs_change_rev_prop2 >+ (db->resource->info->repos->repos, >+ db->resource->info->root.rev, >+ db->resource->info->repos->username, >+ propname, value, >+ db->authz_read_func, >+ db->authz_read_baton, >+ db->resource->pool); > else > serr = svn_repos_fs_change_node_prop(db->resource->info->root.root, > get_repos_path(db->resource->info), >@@ -171,6 +184,7 @@ > int ro, dav_db **pdb) > { > dav_db *db; >+ dav_svn_authz_read_baton *arb; > > /* Some resource types do not have deadprop databases. Specifically: > REGULAR, VERSION, and WORKING resources have them. (SVN does not >@@ -205,6 +219,13 @@ > /* ### temp hack */ > db->work = svn_stringbuf_ncreate("", 0, db->p); > >+ /* make our path-based authz callback available to svn_repos_* funcs. */ >+ arb = apr_pcalloc(p, sizeof(*arb)); >+ arb->r = resource->info->r; >+ arb->repos = resource->info->repos; >+ db->authz_read_baton = arb; >+ db->authz_read_func = dav_svn_authz_read; >+ > /* ### use RO and node's mutable status to look for an error? */ > > *pdb = db; >@@ -364,10 +385,14 @@ > not a working resource! But this is how we currently > (hackily) allow the svn client to change unversioned rev > props. See issue #916. */ >- serr = svn_repos_fs_change_rev_prop(db->resource->info->repos->repos, >- db->resource->info->root.rev, >- db->resource->info->repos->username, >- propname, NULL, db->resource->pool); >+ serr = svn_repos__fs_change_rev_prop2 >+ (db->resource->info->repos->repos, >+ db->resource->info->root.rev, >+ db->resource->info->repos->username, >+ propname, NULL, >+ db->authz_read_func, >+ db->authz_read_baton, >+ db->resource->pool); > else > serr = svn_repos_fs_change_node_prop(db->resource->info->root.root, > get_repos_path(db->resource->info), >@@ -402,9 +427,12 @@ > serr = svn_fs_txn_prop(&propval, db->resource->info->root.txn, > propname, db->p); > else >- serr = svn_fs_revision_prop(&propval, db->resource->info->repos->fs, >- db->resource->info->root.rev, >- propname, db->p); >+ serr = svn_repos__fs_revision_prop(&propval, >+ db->resource->info->repos->repos, >+ db->resource->info->root.rev, >+ propname, >+ db->authz_read_func, >+ db->authz_read_baton, db->p); > else > serr = svn_fs_node_prop(&propval, db->resource->info->root.root, > get_repos_path(db->resource->info), >@@ -457,10 +485,13 @@ > db->resource->info->root.txn, > db->p); > else >- serr = svn_fs_revision_proplist(&db->props, >- db->resource->info->repos->fs, >- db->resource->info->root.rev, >- db->p); >+ serr = svn_repos__fs_revision_proplist >+ (&db->props, >+ db->resource->info->repos->repos, >+ db->resource->info->root.rev, >+ db->authz_read_func, >+ db->authz_read_baton, >+ db->p); > } > else > { >Index: subversion/mod_dav_svn/liveprops.c >=================================================================== >--- subversion/mod_dav_svn/liveprops.c (revision 10978) >+++ subversion/mod_dav_svn/liveprops.c (working copy) >@@ -31,7 +31,10 @@ > #include "svn_dav.h" > #include "svn_md5.h" > >+/* Private header included just for 1.0.7 revprops security fix. */ >+#include "../libsvn_repos/repos.h" > >+ > /* > ** The namespace URIs that we use. This list and the enumeration must > ** stay in sync. >@@ -117,6 +120,7 @@ > apr_pool_t *p = resource->info->pool; > const dav_liveprop_spec *info; > int global_ns; >+ dav_svn_authz_read_baton arb; > svn_error_t *serr; > > /* >@@ -130,6 +134,10 @@ > if (!resource->exists) > return DAV_PROP_INSERT_NOTSUPP; > >+ /* Build a baton for path-based authz function (dav_svn_authz_read) */ >+ arb.r = resource->info->r; >+ arb.repos = resource->info->repos; >+ > /* ### we may want to respond to DAV_PROPID_resourcetype for PRIVATE > ### resources. need to think on "proper" interaction with mod_dav */ > >@@ -218,10 +226,11 @@ > } > > /* Get the date property of the created revision. */ >- serr = svn_fs_revision_prop(&last_author, >- resource->info->repos->fs, >- committed_rev, >- SVN_PROP_REVISION_AUTHOR, p); >+ serr = svn_repos__fs_revision_prop(&last_author, >+ resource->info->repos->repos, >+ committed_rev, >+ SVN_PROP_REVISION_AUTHOR, >+ dav_svn_authz_read, &arb, p); > if (serr != NULL) > { > /* ### what to do? */ >@@ -651,7 +660,11 @@ > svn_string_t *committed_date = NULL; > svn_error_t *serr; > apr_time_t timeval_tmp; >+ dav_svn_authz_read_baton arb; > >+ arb.r = resource->info->r; >+ arb.repos = resource->info->repos; >+ > if ((datestring == NULL) && (timeval == NULL)) > return 0; > >@@ -677,10 +690,12 @@ > } > > /* Get the svn:date property of the CR */ >- serr = svn_fs_revision_prop(&committed_date, >- resource->info->repos->fs, >- committed_rev, >- SVN_PROP_REVISION_DATE, pool); >+ serr = svn_repos__fs_revision_prop(&committed_date, >+ resource->info->repos->repos, >+ committed_rev, >+ SVN_PROP_REVISION_DATE, >+ dav_svn_authz_read, &arb, >+ pool); > if (serr != NULL) > return 1; > >Index: subversion/libsvn_repos/fs-wrap.c >=================================================================== >--- subversion/libsvn_repos/fs-wrap.c (revision 10978) >+++ subversion/libsvn_repos/fs-wrap.c (working copy) >@@ -138,7 +138,7 @@ > > > >-/*** Property change wrappers ***/ >+/*** Property wrappers ***/ > > /* Validate that property NAME is valid for use in a Subversion > repository. */ >@@ -182,7 +182,115 @@ > } > > >+/* A revision's changed paths are either all readable, all unreadable, >+ or a mixture of the two. */ >+enum rev_readability_level >+{ >+ rev_readable = 1, >+ rev_partially_readable, >+ rev_unreadable >+}; >+ >+ >+/* Helper func: examine the changed-paths of REV in FS using >+ AUTHZ_READ_FUNC. Set *CAN_READ to one of the three >+ readability_level enum values. Use POOL for invoking the authz func. */ >+static svn_error_t * >+get_readability (int *can_read, >+ svn_fs_t *fs, >+ svn_revnum_t rev, >+ svn_repos_authz_func_t authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool) >+{ >+ svn_fs_root_t *root; >+ apr_hash_t *changes; >+ apr_hash_index_t *hi; >+ apr_pool_t *subpool = svn_pool_create (pool); >+ svn_boolean_t found_readable = FALSE, found_unreadable = FALSE; >+ >+ SVN_ERR (svn_fs_revision_root (&root, fs, rev, pool)); >+ SVN_ERR (svn_fs_paths_changed (&changes, root, pool)); >+ >+ if (apr_hash_count (changes) == 0) >+ { >+ /* No paths changed in this revision? Uh, sure, I guess the >+ revision is readable, then. */ >+ *can_read = rev_readable; >+ return SVN_NO_ERROR; >+ } >+ >+ for (hi = apr_hash_first (pool, changes); hi; hi = apr_hash_next (hi)) >+ { >+ const void *key; >+ const char *path; >+ svn_boolean_t readable; >+ >+ svn_pool_clear (subpool); >+ >+ apr_hash_this (hi, &key, NULL, NULL); >+ path = (const char *) key; >+ >+ SVN_ERR (authz_read_func (&readable, root, path, >+ authz_read_baton, subpool)); >+ if (readable) >+ found_readable = TRUE; >+ else >+ found_unreadable = TRUE; >+ } >+ >+ svn_pool_destroy (subpool); >+ >+ if (found_unreadable && (! found_readable)) >+ *can_read = rev_unreadable; >+ else if (found_readable && (! found_unreadable)) >+ *can_read = rev_readable; >+ else /* found both readable and unreadable */ >+ *can_read = rev_partially_readable; >+ >+ return SVN_NO_ERROR; >+} >+ >+ > svn_error_t * >+svn_repos__fs_change_rev_prop2 (svn_repos_t *repos, >+ svn_revnum_t rev, >+ const char *author, >+ const char *name, >+ const svn_string_t *new_value, >+ svn_repos_authz_func_t authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool) >+{ >+ svn_string_t *old_value; >+ int readability = rev_readable; >+ >+ if (authz_read_func) >+ SVN_ERR (get_readability (&readability, repos->fs, rev, >+ authz_read_func, authz_read_baton, pool)); >+ if (readability == rev_readable) >+ { >+ SVN_ERR (validate_prop (name, pool)); >+ SVN_ERR (svn_fs_revision_prop (&old_value, repos->fs, rev, name, pool)); >+ SVN_ERR (svn_repos__hooks_pre_revprop_change (repos, rev, author, name, >+ new_value, pool)); >+ SVN_ERR (svn_fs_change_rev_prop (repos->fs, rev, name, new_value, pool)); >+ SVN_ERR (svn_repos__hooks_post_revprop_change (repos, rev, author, >+ name, old_value, pool)); >+ } >+ else /* rev is either unreadable or only partially readable */ >+ { >+ return svn_error_createf >+ (APR_EGENERAL, NULL, >+ "Write denied: not authorized to read all of revision %ld.", rev); >+ } >+ >+ return SVN_NO_ERROR; >+} >+ >+ >+ >+svn_error_t * > svn_repos_fs_change_rev_prop (svn_repos_t *repos, > svn_revnum_t rev, > const char *author, >@@ -190,23 +298,106 @@ > const svn_string_t *new_value, > apr_pool_t *pool) > { >- svn_string_t *old_value; >+ return svn_repos__fs_change_rev_prop2 (repos, rev, author, name, new_value, >+ NULL, NULL, pool); >+} > >- SVN_ERR (validate_prop (name, pool)); >- SVN_ERR (svn_fs_revision_prop (&old_value, repos->fs, rev, name, pool)); >- SVN_ERR (svn_repos__hooks_pre_revprop_change (repos, rev, author, name, >- new_value, pool)); >- SVN_ERR (svn_fs_change_rev_prop (repos->fs, rev, name, new_value, pool)); >- SVN_ERR (svn_repos__hooks_post_revprop_change (repos, rev, author, >- name, old_value, pool)); > >+ >+svn_error_t * >+svn_repos__fs_revision_prop (svn_string_t **value_p, >+ svn_repos_t *repos, >+ svn_revnum_t rev, >+ const char *propname, >+ svn_repos_authz_func_t authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool) >+{ >+ int readability = rev_readable; >+ >+ if (authz_read_func) >+ SVN_ERR (get_readability (&readability, repos->fs, rev, >+ authz_read_func, authz_read_baton, pool)); >+ >+ if (readability == rev_unreadable) >+ { >+ /* Property? What property? */ >+ *value_p = NULL; >+ } >+ else if (readability == rev_partially_readable) >+ { >+ /* Only svn:author and svn:date are fetchable. */ >+ if ((strncmp (propname, SVN_PROP_REVISION_AUTHOR, >+ strlen(SVN_PROP_REVISION_AUTHOR)) != 0) >+ && (strncmp (propname, SVN_PROP_REVISION_DATE, >+ strlen(SVN_PROP_REVISION_DATE)) != 0)) >+ *value_p = NULL; >+ >+ else >+ SVN_ERR (svn_fs_revision_prop (value_p, repos->fs, >+ rev, propname, pool)); >+ } >+ else /* wholly readable revision */ >+ { >+ SVN_ERR (svn_fs_revision_prop (value_p, repos->fs, rev, propname, pool)); >+ } >+ > return SVN_NO_ERROR; > } > > > >+svn_error_t * >+svn_repos__fs_revision_proplist (apr_hash_t **table_p, >+ svn_repos_t *repos, >+ svn_revnum_t rev, >+ svn_repos_authz_func_t authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool) >+{ >+ int readability = rev_readable; > >+ if (authz_read_func) >+ SVN_ERR (get_readability (&readability, repos->fs, rev, >+ authz_read_func, authz_read_baton, pool)); > >+ if (readability == rev_unreadable) >+ { >+ /* Return an empty hash. */ >+ *table_p = apr_hash_make (pool); >+ } >+ else if (readability == rev_partially_readable) >+ { >+ apr_hash_t *tmphash; >+ svn_string_t *value; >+ >+ /* Produce two property hashtables, both in POOL. */ >+ SVN_ERR (svn_fs_revision_proplist (&tmphash, repos->fs, rev, pool)); >+ *table_p = apr_hash_make (pool); >+ >+ /* If they exist, we only copy svn:author and svn:date into the >+ 'real' hashtable being returned. */ >+ value = apr_hash_get (tmphash, SVN_PROP_REVISION_AUTHOR, >+ APR_HASH_KEY_STRING); >+ if (value) >+ apr_hash_set (*table_p, SVN_PROP_REVISION_AUTHOR, >+ APR_HASH_KEY_STRING, value); >+ >+ value = apr_hash_get (tmphash, SVN_PROP_REVISION_DATE, >+ APR_HASH_KEY_STRING); >+ if (value) >+ apr_hash_set (*table_p, SVN_PROP_REVISION_DATE, >+ APR_HASH_KEY_STRING, value); >+ } >+ else /* wholly readable revision */ >+ { >+ SVN_ERR (svn_fs_revision_proplist (table_p, repos->fs, rev, pool)); >+ } >+ >+ return SVN_NO_ERROR; >+} >+ >+ > > /* > * vim:ts=4:sw=4:expandtab:tw=80:fo=tcroq >Index: subversion/libsvn_repos/repos.h >=================================================================== >--- subversion/libsvn_repos/repos.h (revision 10978) >+++ subversion/libsvn_repos/repos.h (working copy) >@@ -164,6 +164,45 @@ > apr_pool_t *pool); > > >+/* Included only for 1.0.7 security fix. Allows use of >+ authz_read_func, which is supplied by mod_dav_svn. */ >+svn_error_t * >+svn_repos__fs_revision_prop (svn_string_t **value_p, >+ svn_repos_t *repos, >+ svn_revnum_t rev, >+ const char *propname, >+ svn_repos_authz_func_t >+ authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool); >+ >+ >+/* Included only for 1.0.7 security fix. Allows use of >+ authz_read_func, which is supplied by mod_dav_svn. */ >+svn_error_t * >+svn_repos__fs_revision_proplist (apr_hash_t **table_p, >+ svn_repos_t *repos, >+ svn_revnum_t rev, >+ svn_repos_authz_func_t >+ authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool); >+ >+ >+/* Included only for 1.0.7 security fix. Allows use of >+ authz_read_func, which is supplied by mod_dav_svn. */ >+svn_error_t * >+svn_repos__fs_change_rev_prop2 (svn_repos_t *repos, >+ svn_revnum_t rev, >+ const char *author, >+ const char *name, >+ const svn_string_t *new_value, >+ svn_repos_authz_func_t >+ authz_read_func, >+ void *authz_read_baton, >+ apr_pool_t *pool); >+ >+ > #ifdef __cplusplus > } > #endif /* __cplusplus */
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=23675">View the attachment on a separate page</a>.</b>
Actions:
View
|
Diff
Attachments on
bug 60610
:
23673
|
23674
| 23675