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

[Rui Paulo]

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.

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

--
Rui Paulo