Stefan Fuhrmann <stef...@apache.org> writes: > Within libsvn_repos get_log functionality, pass the list of wanted revprops > around as an array of svn_string_t* instead of const char*. The added length > info allows for more effient functions to be used. Do that.
[...] > - char *name = APR_ARRAY_IDX(revprops, i, char *); > - svn_string_t *value = svn_hash_gets(r_props, name); > - if (censor_revprops > - && !(strcmp(name, SVN_PROP_REVISION_AUTHOR) == 0 > - || strcmp(name, SVN_PROP_REVISION_DATE) == 0)) > - /* ... but we can only return author/date. */ > - continue; [...] > + const svn_string_t *name > + = APR_ARRAY_IDX(revprops, i, const svn_string_t *); > + svn_string_t *value > + = apr_hash_get(r_props, name->data, name->len); > + if (censor_revprops > + && !(strncmp(name->data, SVN_PROP_REVISION_AUTHOR, > + name->len) == 0 > + || strncmp(name->data, SVN_PROP_REVISION_DATE, > + name->len) == 0)) > + /* ... but we can only return author/date. */ > + continue; As it turns out, this particular micro-optimization makes a data leak possible. This is not a real security issue, as the change happened on trunk and didn't become part of any released version. Still, I think that we should fix this prior to making 1.9 public. I don't know what are the performance implications of using strncmp() in favor of strcmp(), but the new check will not censor properties like 's', 'sv', ... 'svn:a', 'svn:d' and others. This means that we might incorrectly leak these revision properties for partially visible revisions. Subversion 1.8.x only outputs svn:date / svn:author when perfoming log requests for partially visible revisions, and *all* other revision properties are censored out, but with this changeset this is no longer true. I committed a failing test in r1658406. As for fixing this issue, I think that we should entirely revert this changeset. Thoughts? Regards, Evgeny Kotkov
