svn commit: r278479 - in head: etc sys/kern

Mateusz Guzik mjguzik at gmail.com
Tue Feb 10 10:01:56 UTC 2015


On Mon, Feb 09, 2015 at 06:57:36PM -0800, Rui Paulo wrote:
> On Feb 9, 2015, at 18:43, Mateusz Guzik <mjguzik at gmail.com> wrote:
> > 
> > On Mon, Feb 09, 2015 at 11:13:51PM +0000, Rui Paulo wrote:
> >> +notify 10 {
> >> +	match "system"          "kernel";
> >> +	match "subsystem"       "signal";
> >> +	match "type"            "coredump";
> >> +	action "logger $comm $core";
> >> +};
> >> +
> >> */
> >> 
> > [..]
> >> +	if (vn_fullpath_global(td, p->p_textvp, &fullpath, &freepath) != 0)
> >> +		goto out;
> >> +	snprintf(data, len, "comm=%s", fullpath);
> > 
> > I cannot test it right now, but it looks like immediate privilege
> > escalation.
> > 
> > Path is not sanitized in any way and devd passes it to 'sh -c'.
> > 
> > So a file named "a.out; /bin/id; meh" or so should result in execution
> > of aforementioned /bin/id.
> 
> Well, you can't have a file name with "/" but you're right.
> 

I mean the whole path. You are resolving the name along with all dirs,
so it's just a matter of some mkdirs.

> > Another note is that currently devctl is record oriented, but this may
> > change at some point and free form userspace text could be used to forge
> > new events.
> > 
> > As such is trongly suggest we sanitize this somehow. Maybe a base64 or
> > something.
> 
> I was trying hard to avoid this issue in unpublished my crash helper, but I forgot that devd runs execl("sh -c", ....);  :-(
> 
> It might just be easier to inspect the path names and allow only [a-z][A-Z][0-9] and '/' before sending the devctl message.
> 

I'm pretty sure sooner or later people will want something with a space,
so I would prefer a reasonably complete solution.

A hack like the one yu mention should suffice fr now though (with the
addition of a dot).

-- 
Mateusz Guzik <mjguzik gmail.com>


More information about the svn-src-all mailing list