You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2006/04/28 00:23:18 UTC
[PATCH] stricter parsing of values in authz files
Ok, so since we can't modify the config parser, here's a patch that
makes us a bit stricter about the parsing of the values in
authorization config files.
I'm not overly thrilled with this, since it doesn't actually error out
when an unexpected character is found (it just stops parsing). The
enumeration functions don't actually allow you to return an error, so
currently the only way to do so is to stash an error in the baton and
look at it later, which seems kind of hacky. Plus, I'm not positive
if we'd want to explicitly error out, although silently accepting
invalid input seems bad as well.
-garrett
[[[
Make authz parsing more strict, stopping once it sees characters that are
not valid directives.
* subversion/libsvn_repos/authz.c
(authz_parse_line): Instead of doing a simple strchr to find r and w
characters in the value use a more strict approach that stops us from
running off into comments.
]]]
Re: [PATCH] stricter parsing of values in authz files
Posted by Jesper Steen Møller <je...@selskabet.org>.
Peter N. Lundblad wrote:
> Garrett Rooney writes:
>
> > +#include <apr_lib.h> /* for apr_isspace() */
>
> Hmmm, which encoding is the authz file supposed to be in *shrug*. I
> ask because apr_isspace is locale-dependent. If it is UTF8, use
> svn_ctype.h instead.
>
As the a Chinese user a few weeks back didn't expect, the authz files
(and all other Subversion config files) are used raw as UTF-8 - mostly
untouched 8-bit byte sequences are copied into in-memory structures.
Also, on AS/400 the files always read as UTF-8 regardless of the CCSID
set on the file.
I guess this could be documented somewhere.
Also: On Windows clients, there is a charset mismatch between the byte
sequences read from config files (UTF8) and the values read from the
registry (using the RegXxxxA APIs), which are 8-bit ANSI codepage values.
-Jesper
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] stricter parsing of values in authz files
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/28/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> Garrett Rooney writes:
> > Index: subversion/libsvn_repos/authz.c
> > ===================================================================
> > --- subversion/libsvn_repos/authz.c (revision 19470)
> > +++ subversion/libsvn_repos/authz.c (working copy)
> > @@ -22,6 +22,7 @@
> >
> > #include <apr_pools.h>
> > #include <apr_file_io.h>
> > +#include <apr_lib.h> /* for apr_isspace() */
>
> Hmmm, which encoding is the authz file supposed to be in *shrug*. I
> ask because apr_isspace is locale-dependent. If it is UTF8, use
> svn_ctype.h instead.
>
> Else, +1.
Works for me, see r19471.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] stricter parsing of values in authz files
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Garrett Rooney writes:
> Index: subversion/libsvn_repos/authz.c
> ===================================================================
> --- subversion/libsvn_repos/authz.c (revision 19470)
> +++ subversion/libsvn_repos/authz.c (working copy)
> @@ -22,6 +22,7 @@
>
> #include <apr_pools.h>
> #include <apr_file_io.h>
> +#include <apr_lib.h> /* for apr_isspace() */
Hmmm, which encoding is the authz file supposed to be in *shrug*. I
ask because apr_isspace is locale-dependent. If it is UTF8, use
svn_ctype.h instead.
Else, +1.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] stricter parsing of values in authz files
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/27/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> Garrett Rooney writes:
> > I'm not overly thrilled with this, since it doesn't actually error out
> > when an unexpected character is found (it just stops parsing). The
> > enumeration functions don't actually allow you to return an error, so
> > currently the only way to do so is to stash an error in the baton and
> > look at it later, which seems kind of hacky.
>
> What's hacky about that? But...
>
> > Plus, I'm not positive
> > if we'd want to explicitly error out, although silently accepting
> > invalid input seems bad as well.
> >
> Why would we want to silently accept invalid input in a file that controls
> authorization? I suggest putting this check in authz_validate_rule, where
> we currently validate that a rule don't refer to a non-existent group.
You've convinced me. How about this?
-garrett
[[[
Make sure that the authz rules don't contain any unexpected characters.
* subversion/libsvn_repos/authz.c
Include apr_lib.h.
(authz_validate_rule): Check that there are no unexpected characters in
the rule.
* subversion/tests/cmdline/authz_tests.py
(broken_authz_file): New test.
(test_list): Run new test.
]]]
Re: [PATCH] stricter parsing of values in authz files
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Garrett Rooney writes:
> I'm not overly thrilled with this, since it doesn't actually error out
> when an unexpected character is found (it just stops parsing). The
> enumeration functions don't actually allow you to return an error, so
> currently the only way to do so is to stash an error in the baton and
> look at it later, which seems kind of hacky.
What's hacky about that? But...
> Plus, I'm not positive
> if we'd want to explicitly error out, although silently accepting
> invalid input seems bad as well.
>
Why would we want to silently accept invalid input in a file that controls
authorization? I suggest putting this check in authz_validate_rule, where
we currently validate that a rule don't refer to a non-existent group.
Best Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org