On Wed, Aug 7, 2013 at 9:04 PM, Ben Reser <[email protected]> wrote:
> An attacker can open a connection and starting making requests
> to fill up a servers memory. As long as they keep making authz
> requests as the same user on the same connection they will be able to
> increase the server usage everytime we do authz against an entry that
> exists in the authz file and hasn't been cached.
>
Err... the cache apr_hash_t by construction cannot contain keys (i.e.paths)
that are not in the authz file also, so is bounded by the size of the
corresponding svn_config_t. In fact, one could precompute the maximal
cache on the first call to svn_repos_authz_check_access() by
iterating over all paths in svn_config_t.
... but if there is a proxy that proxy might be
> serving multiple clients connections on the same connection to the SVN
> server. In these cases the authz cache is going to be cleared
> regularly for that connection and will actually hurt performance.
>
I see your point. "Clearing the cache" here means a single svn_clear_pool()
call...
> if (!(user && authz->cached_user
> > && strcmp(user, authz->cached_user) == 0))
> > {
> > svn_pool_clear(authz->pool);
> > authz->cache = NULL;
> > authz->cached_user = user ? apr_pstrdup(authz->pool, user) : NULL;
> > }
>
> This code will incorrectly clear the cache on every anonymous access
> even when user and authz->cached_user are both NULL.
>
Good spotting. I'll revert to the previous
if (user) ...
else if (authz->cached_user) ...
logic. So much for going for elegance :)
Cheers, Roderich
