On Wed, Jan 21, 2015 at 1:15 PM, Richard W.M. Jones <[email protected]> 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
_______________________________________________ augeas-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/augeas-devel
