tech-security archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: realpath(3)



    Date:        Thu, 25 May 2023 15:30:12 +0200
    From:        tlaronde%polynum.com@localhost
    Message-ID:  <ZG9i5LcY9XfxlStC%polynum.com@localhost>

  | I will beg to differ.

You can do that, but nothing is likely to change because of that.

  | This example, IMO, shows that using the same variable to hold
  | whether the canonical result to be used, or an error,

It isn't an error, as such, it is just that the canonical path
is being gradually constructed, until at some point it fails,
and no more can be done.   Eg: if the arg were "/usr/bin/gibberish/whatever"
There's no "gibberish" (I am assuming) in /usr/bin so there's no way to
resolve what "whatever" might be inside a directory which doesn't exist.
So realpath() returns NULL, but "/usr/bin/gibberish" (which is as far
as it had progresses - assuming that none of those components are symlinks)
is where we stopped.

It would be possible to build the path elsewhere, then copy it back later,
but why?   And how would that help, all that would mean if an error occurs
is that different garbage would be in the buffer, which is of no help to
anyone.   As it is, when we look at errno, and see ENOENT or ENOTDIR or
whatever error actually happened, we can (in the NetBSD implementation,
which I suspect is more a BSD implementation) we can see which path
actually provoked that error, rather than being left in the dark.

  | relying on programmers to test correctly the return
  | status of a routine, is dangerous.

If that's your attitude you should stop writing any C code, as that's
how it is done everywhere.  Functions that can fail indicate that in
one manner or other, and it is the responsibility of the caller to check.

And yes, sloppy code sometimes doesn't - and when the function in question
rarely fails, that can lead to bugs that remain latent for a long time.

But that's just the way it is.   There's too much of this for there to be
any hope of fixing it.

Here mount should be fixed, realpath(3) is fine as it is.

  | IMHO, realpath(3) should be a wrapper around a xplpath(3) ("explicate
  | path", or whatever name seems fitting for an english native
  | speaker---which I'm obviously not) that would take a char ** as
  | third argument whose value, if not NULL, will be defined so to
  | point after the part successfully processed (hence, in case of
  | failure, one can know where the routine choked).

How the implementation is done isn't the issue, but what you describe
is not what happens now.   What is in the buffer (in the error case)
need not have even a passing resemblance to what was in the original
path (obviously that designates what will happen, but the strings can
be quite different - that's the point of realpath).   You can't just
return a (very useful) pointer to somewhere in the original path, as
the component with the error might not exist there.

  | And would it be good then to define, in case of error, resolvedname
  | with a name (is the empty string safe?) that for sure will make
  | any operation on this path fail?

Since one of the things one frequently does with the result of realpath(3)
is to join it to another string, that would be difficult to imagine.
You're assuming that the only reason is to use the result as is (which is
what mount does, as it wants the mount to be the canonical name of the
mount point, not whatever twisty path the user happened to use to specify
it) but for other uses, that's not true.   You can look at uses of
realpath(3) in realpath(1) if you want to see it used quite differently.
(I just noticed a minor bug in the latter that I will be fixing soon.)

kre



Home | Main Index | Thread Index | Old Index