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

[Richard W.M. Jones]

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"):

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
  )

or:

 let repos =
   g#aug_match "/files/etc/yum.repos.d/*/*[baseurl =~ regexp('https?://([^/]+\\.)?vmware\\.com/.*')]" in
  let repos = Array.to_list repos in
  List.iter (
    fun repo ->
      g#aug_set (repo ^ "/enabled") "0";
      g#aug_save ()
  ) repos;

or:

 let paths = g#aug_match ("/files" ^ xorg_conf ^ "/Device/Driver") in
 Array.iter (
   fun path ->
     g#aug_set path video_driver;
     updated := true
 ) paths;

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).

> 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?

> I just revised my patch series since there was one place internally (when
> Augeas deletes a file) where that would lead to trouble.
> 
> The new aug_escape_name API also assumes it gets part of a path; if you
> give it a full path it will faithfully escape '/', too. Probably what you
> expected, but worth pointing out.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org