[augeas-devel] escaping special characters in the path sent to aug_get()

[David Lutterkort]

On Wed, Jan 21, 2015 at 1:15 PM, Richard W.M. Jones <rjones at redhat.com>
wrote:

> On Wed, Jan 21, 2015 at 12:59:40PM -0800, David Lutterkort wrote:
> > The thing that makes me nervous the most about this change is that
> > it changes the paths that people get back from Augeas, especially
> > from aug_match.
>
> [I think the first thing to say is that we only care about the API, not
> about augtool.]
>
> I've probably not understood the full implications of this.
>
> Programs like libvirt and virt-v2v use the aug_match a lot, and in
> some cases pass those strings back to aug_get, aug_set, aug_rm.  There
> are many examples of this in the following file (search for
> "aug_match"):
>

With this change, taking paths returned from aug_match and feeding them
into aug_get, aug_set, aug_rm etc. now actually becomes safe: before,
aug_match would return paths that really needed escaping, but weren't so
that passing them to aug_get could fail. This will no longer be an issue.

https://github.com/libguestfs/libguestfs/blob/master/v2v/convert_linux.ml
>
> For example:
>
>  let expr =
>    sprintf "/file/etc/sysconfig/kernel/%s/value[. = '%s']"
>      var xen_mod in
>   let entries = g#aug_match expr in
>   let entries = Array.to_list entries in
>   if entries <> [] then (
>     List.iter (fun e -> ignore (g#aug_rm e)) entries;
>     modified := true
>   )
>

Yes, this will now work, assuming you replace 'var' in there with
'(g#aug_escape_name var)'


> Our real concern is where strings get interpolated into an Augeas
> expression, especially if those string come from untrusted user input
> (which is not the case in that file, but could be in general).
>

As long as these strings go through aug_escape_name before sticking them
into a path, all should be well.


>  > While those can now be directly fed to aug_get, they can no longer
> > be used to find the underlying file directly. Not sure if that will
> > cause problems for anybody.
>
> I'm not really sure what this means.  What is "underlying file" in
> this context?  Would it affect code like the above?  Can you give an
> example of a problem case?


Sorry, this was a bit obtuse: before this change, you could take a path
returned by aug_match, strip off '/files' from the beginning and use the
rest as the path to the actual file in the filesystem. After this change,
that's no longer the case. If you have a file '/etc/weird-[a]' in the
filesystem, an aug_match for something in that file will return
'/files/etc/weird-\[a\]' - you can now pass that back to aug_get, but you
can't use that to open the actual file (because of the extra '\' in there).

David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/augeas-devel/attachments/20150121/fdf5ffac/attachment.htm>