popa3d-users - Re: popa3d-vname, allowing hierarchies for "domain:directory" in vnamemap

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 9 Nov 2010 13:35:32 +0300
From: Solar Designer <solar@...nwall.com>
To: popa3d-users@...ts.openwall.com
Subject: Re: popa3d-vname, allowing hierarchies for "domain:directory" in vnamemap

Hi Andy,

On Tue, Nov 09, 2010 at 06:13:07PM +0800, Andy Sy wrote:
>   "mydomain.com:anotherdir/mydomain"
[...]
> I wrote the attached patch to allow the above to happen,
> but is this safe?

I didn't review it in full context, but it looks like you're lucky
as it relates to "address" since the directory pathname comes from a
trusted config file only, but not as it relates to "user".

There doesn't appear to be any reason for you to remove the check of
"user", so I suggest that you fix the (patched) code in this respect.

As to "address", I recommend that rather than completely remove the
check for slash you replace it with a check preventing traversal to
upper-level directories.

Something like:

	if (strchr(user, '/') ||
	    !strcmp(user, "..") ||
	    strstr(address, ".."))
		return NULL;

...and you don't need vname_lookup_fail.

This is completely untested, use at your own risk.

Alexander