undefined | Better HN

0 pointsTheDong4y ago0 comments

I mean, can you blame them?

The docs for filepath.Clean state the following:

    ...
    3. Eliminate each inner .. path name element (the parent directory)
       along with the non-.. element that precedes it.
    4. Eliminate .. elements that begin a rooted path:
       that is, replace "/.." by "/" at the beginning of a path,
       assuming Separator is '/'.

There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

A quick reading of 3 and 4 will make you assume that a path has no ".."s after being "Clean"d. If you actually think about it more, you'll realize that of course it will leave ".." at the beginning of relative paths, but the docs do not make it clear, and I can understand why a programmer might reach for this.

There's no clearer function to reach for either, like "filepath.DirectoryContains(parent, path) bool".

0 comments

gpderetta4y ago

> There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

It seems to me that both of those guarantees are something that would need to be explicitly stated if provided, and one wouldn't just assume that they hold by omission. In general if the docs were to list all postconditions that are not guaranteed to hold, it would take a while to write them :).

acdha4y ago

Just to be clear, I’m not blaming them. I should have clarified that I was thinking along the lines of “how could the tool/documentation have made it easier to recognize that gap?” — and I especially agree with your suggestion that there could be a solid library improvement with a purpose-built function which is clearly identified as the right way to address this relatively common need.

jacquesm4y ago

A good example case for why testing your assumptions is usually a good thing.

gpderetta4y ago

Actually no. Testing even a fairly comprehensive set of test cases could have given a false sense of security and possibly missed some actual vulnerable cases.

Best thing would have been to either implement a function that does give the required guarantees or look for an existing library that explicitly does.

j / k navigate · click thread line to collapse

0 pointsTheDong4y ago0 comments

I mean, can you blame them?

The docs for filepath.Clean state the following:

    ...
    3. Eliminate each inner .. path name element (the parent directory)
       along with the non-.. element that precedes it.
    4. Eliminate .. elements that begin a rooted path:
       that is, replace "/.." by "/" at the beginning of a path,
       assuming Separator is '/'.

There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

There's no clearer function to reach for either, like "filepath.DirectoryContains(parent, path) bool".

0 comments

gpderetta4y ago

> There is no mention that this function cannot be used for security, or that it does not safely ensure a path can't refer to a parent directory.

acdha4y ago

jacquesm4y ago

A good example case for why testing your assumptions is usually a good thing.

gpderetta4y ago

Actually no. Testing even a fairly comprehensive set of test cases could have given a false sense of security and possibly missed some actual vulnerable cases.

Best thing would have been to either implement a function that does give the required guarantees or look for an existing library that explicitly does.

j / k navigate · click thread line to collapse