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

[Andy Sy]

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

[Solar Designer]

Hi Andy,

Post by Andy Sy
"mydomain.com:anotherdir/mydomain"

[...]

Post by Andy Sy
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

[Andy Sy]

Hi Alexander,

Post by Solar Designer
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.
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

[Solar Designer]

Andy,

Post by Andy Sy
Was able to drastically simplify the patch by just replacing
if ( strchr(user, '/') || ...
if ( strstr(address, "..") || ...

You need to keep the user check as well!

Alexander

[Andy Sy]

Post by Solar Designer

Post by Andy Sy
Was able to drastically simplify the patch by just replacing
if ( strchr(user, '/') || ...
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