headers
Andy Sy | 9 Nov 2010 11:13
Favicon
Gravatar

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

Hi all,

I am using hhg's patch at

   http://hhg.to/popa3d/popa3d-0.6.3-vname-2.diff

but it doesn't seem to support directory hierarchies for
directory values in vnamemap. e.g.

   "mydomain.com:mydomain"

works, but

   "mydomain.com:anotherdir/mydomain"

won't.

I wrote the attached patch to allow the above to happen,
but is this safe?

Note that the patch can be applied to unpatched 1.0.2
and result in compilable virtual.c, but only makes
sense when popa3d-0.6.3-vname-2.diff is also applied.

- Andy

============================
http://webmechs.com/webpress
The Webmechs Webpress Blog
(Continue reading)

Permalink | Reply | 4 comments |
headers
Solar Designer | 9 Nov 2010 11:35
Favicon

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.
(Continue reading)

Permalink | Reply | 3 comments |
headers
Andy Sy | 9 Nov 2010 12:24
Favicon
Gravatar

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

Hi Alexander,

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

Was able to drastically simplify the patch by just replacing

   if ( strchr(user, '/') || ...

with:

   if ( strstr(address, "..") || ...

The above seems to work fine.

- Andy
(Continue reading)

Permalink | Reply | 2 comments |
headers
Solar Designer | 9 Nov 2010 12:49
Favicon

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

Andy,

On Tue, Nov 09, 2010 at 07:24:25PM +0800, Andy Sy wrote:
> Was able to drastically simplify the patch by just replacing
> 
>   if ( strchr(user, '/') || ...
> 
> with:
> 
>   if ( strstr(address, "..") || ...

You need to keep the user check as well!

Alexander
Permalink | Reply | 1 comment |
headers
Andy Sy | 9 Nov 2010 13:15
Favicon
Gravatar

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

> On Tue, Nov 09, 2010 at 07:24:25PM +0800, Andy Sy wrote:
>> Was able to drastically simplify the patch by just replacing
>>
>>   if ( strchr(user, '/') || ...
>>
>> with:
>>
>>   if ( strstr(address, "..") || ...
> 
> You need to keep the user check as well!

Yes, sorry, typo.  This is how the new line looks:

   if ( strstr(address, '/') || strchr(user, '/')) ...

became:

   if ( strstr(address, "..") || strchr(user, '/'))

- Andy
Permalink | Reply | 0 comments |

Gmane