On 16.02.11 03:26, clipka wrote:
> Am 15.02.2011 05:54, schrieb dickbalaska:
>> I have uploaded a small test to the binary scene-files.
>> It demonstrates that I can not include "../../file0.pov"
>
> Found the bastard! Definite bug - when parsing a file path, POV-Ray attempts
> to eliminate ".." by stripping the preceding component from the path - e.g.
> "foo/bar/../something" becomes "foo/something"; unfortunately, it does this
> even if the previous element happens to be ".." as well. So
> "../../file0.pov" simply becomes "file0.pov".
>
> FAIL.
>
> I'll go fix that. Dick, would you mind entering a report on
> http://bugs.povray.org in the meantime, so this gets listed as a known bug?

STOP, that is no bug, that is by design!

	Thorsten

Post a reply to this message

Am 16.02.2011 18:14, schrieb Thorsten Froehlich:
> On 16.02.11 03:26, clipka wrote:
>> Am 15.02.2011 05:54, schrieb dickbalaska:
>>> I have uploaded a small test to the binary scene-files.
>>> It demonstrates that I can not include "../../file0.pov"
>>
>> Found the bastard! Definite bug - when parsing a file path, POV-Ray
>> attempts
>> to eliminate ".." by stripping the preceding component from the path -
>> e.g.
>> "foo/bar/../something" becomes "foo/something"; unfortunately, it does
>> this
>> even if the previous element happens to be ".." as well. So
>> "../../file0.pov" simply becomes "file0.pov".
>>
>> FAIL.
>>
>> I'll go fix that. Dick, would you mind entering a report on
>> http://bugs.povray.org in the meantime, so this gets listed as a known
>> bug?
>
> STOP, that is no bug, that is by design!

What? You mean that it strips any "../.." ???

That's the only thing I'll change. "foo/.." will still be stripped.

Post a reply to this message

clipka wrote:
>> STOP, that is no bug, that is by design!
> 
> What? You mean that it strips any "../.." ???

One would think that at worst the bug would be "it silently gives the wrong 
result instead of reporting an error."

If the intention is to disallow ".." at the start of the string for security 
purposes, it should give an error rather than point at a different file 
(which would be its own security problem right there).  If it's not 
security-related, I'm really curious how this could be considered a feature.

-- 
Darren New, San Diego CA, USA (PST)
  "How did he die?"   "He got shot in the hand."
     "That was fatal?"
          "He was holding a live grenade at the time."

Post a reply to this message

Thorsten Froehlich <tho### [at] trfde> wrote:
> On 16.02.11 03:26, clipka wrote:
> > I'll go fix that. Dick, would you mind entering a report on
> > http://bugs.povray.org in the meantime, so this gets listed as a known bug?
>
> STOP, that is no bug, that is by design!
>
>  Thorsten

I'll file it as a bug report.  You boys fight about how to fix the new behavior.

...... Although it seems to me that if I turn off the I/O restrictions, it
shouldn't be checking for ../ at all.

...... If only Windows had a chroot().

Thanks,
dik

Post a reply to this message

Am 16.02.2011 20:51, schrieb Darren New:
> clipka wrote:
>>> STOP, that is no bug, that is by design!
>>
>> What? You mean that it strips any "../.." ???
>
> One would think that at worst the bug would be "it silently gives the
> wrong result instead of reporting an error."
>
> If the intention is to disallow ".." at the start of the string for
> security purposes, it should give an error rather than point at a
> different file (which would be its own security problem right there). If
> it's not security-related, I'm really curious how this could be
> considered a feature.

 From the code it is pretty obvious that the original intention is to 
eliminate ".." from paths like "foo/bar/../fnord" by contracting it to 
"foo/../fnord" - no security stuff intended there.

Also note that paths having the form "../foo" are passed unharmed.

Post a reply to this message

Am 16.02.2011 23:28, schrieb clipka:

>  From the code it is pretty obvious that the original intention is to
> eliminate ".." from paths like "foo/bar/../fnord" by contracting it to
> "foo/../fnord" - no security stuff intended there.

Of course that should have been "contracting it to 'foo/fnord'".

Post a reply to this message

clipka <ano### [at] anonymousorg> wrote:
> Am 16.02.2011 20:51, schrieb Darren New:

>  From the code it is pretty obvious that the original intention is to
> eliminate ".." from paths like "foo/bar/../fnord" by contracting it to
> "foo/../fnord" - no security stuff intended there.

What is the point of doing path contraction, if not for security purposes?
Otherwise you are introducing code with no benefit. The OS is going to handle
.../ just fine for you already.

Someone above said "../ at the start of the path".  Don't forget that
foo/../../../../etc/passwd is legal.

dik

Post a reply to this message

Am 17.02.2011 02:07, schrieb dickbalaska:
> clipka<ano### [at] anonymousorg>  wrote:
>> Am 16.02.2011 20:51, schrieb Darren New:
>
>>    From the code it is pretty obvious that the original intention is to
>> eliminate ".." from paths like "foo/bar/../fnord" by contracting it to
>> "foo/../fnord" - no security stuff intended there.
>
> What is the point of doing path contraction, if not for security purposes?
> Otherwise you are introducing code with no benefit. The OS is going to handle
> ..../ just fine for you already.
>
> Someone above said "../ at the start of the path".  Don't forget that
> foo/../../../../etc/passwd is legal.

You may be right there - but I guess we agree that contracting 
"foo/../../fnord" to "foo/fnord" is bogus, whatever reason there may be 
to contract "foo/bar/../fnord" to "foo/fnord".

Post a reply to this message

On 17.02.11 11:08, clipka wrote:
> Am 17.02.2011 02:07, schrieb dickbalaska:
>> clipka<ano### [at] anonymousorg> wrote:
>>> Am 16.02.2011 20:51, schrieb Darren New:
>>
>>> From the code it is pretty obvious that the original intention is to
>>> eliminate ".." from paths like "foo/bar/../fnord" by contracting it to
>>> "foo/../fnord" - no security stuff intended there.
>>
>> What is the point of doing path contraction, if not for security purposes?
>> Otherwise you are introducing code with no benefit. The OS is going to handle
>> ..../ just fine for you already.
>>
>> Someone above said "../ at the start of the path". Don't forget that
>> foo/../../../../etc/passwd is legal.
>
> You may be right there - but I guess we agree that contracting
> "foo/../../fnord" to "foo/fnord" is bogus,

Indeed, that is a bug. If a path cannot be resolved to a concrete relative 
path in the backend alone, an error message should be issued. Note that this 
is actually an extension to 3.6 behavior, as it was never documented that 
relative paths would work at all, let alone walking of the directory 
structure using ../.

> whatever reason there may be to
> contract "foo/bar/../fnord" to "foo/fnord".

The reason is not apparent in 3.7 because there is no code that makes use of 
this feature yet: When you render over a network, you don't want to transmit 
files twice. Hence the code is supposed to resolve paths where applicable to 
make life easier for those that use ../ notation as undocumented 3.6 
feature. The net result is then a cache of files.

     Thorsten

Post a reply to this message

Thorsten Froehlich <tho### [at] trfde> wrote:

> Note that this
> is actually an extension to 3.6 behavior, as it was never documented that
> relative paths would work at all, let alone walking of the directory
> structure using ../.
>
> The reason is not apparent in 3.7 because there is no code that makes use of
> this feature yet:

I found the "never documented that relative paths would work" interesting, so I
looked it up.  You are correct.

But I also found this:
http://robotics.ee.uwa.edu.au/courses/ips205/labs/povray/pov139.htm
which, talking about 3.1 Library_Path, says,
"Up to twenty unique paths may be specified. If you specify the exact same path
twice it is only counts once."
So, perhaps this path-pruning is older than 3.6?

dik

Post a reply to this message