Hi,

getcwd(3) (or __getcwd(2)) could successfully return with invalid
result, due to a missing check on path traversal limit.

__getcwd(2) computes a limit to the max vnode number to traverse. In
short, if your buffer is too small to contains the pathname for N
vnodes, there is no need to read them, and the kernel could return
quicker.

kern/vfs_getcwd.c
   412          /*
   413           * 5th argument here is "max number of vnodes to traverse".
   414           * Since each entry takes up at least 2 bytes in the output
   415           * buffer, limit it to N/2 vnodes for an N byte buffer.
   416           */
   417          error = vfs_getcwd_common(p->p_fd->fd_cdir, NULL, &bp, path, 
len/2,
   418              GETCWD_CHECK_ACCESS, p);
   419


The problem is it didn't check if the loop was interrupted due to found
path or due to limit reached.

kern/vfs_getcwd.c
   304          /*
   305           * This loop will terminate when we hit the root, VOP_READDIR() 
or
   306           * VOP_LOOKUP() fails, or we run out of space in the user 
buffer.
   307           */
   308          do {
...
   370
   371                  lvp = uvp;
   372                  uvp = NULL;
   373                  limit--;
   374
   375          } while ((lvp != rvp) && (limit > 0));
   376
   377  out:
   378
   379          if (bpp)
   380                  *bpp = bp;
   381
   382          if (uvp)
   383                  vput(uvp);
   384
   385          if (lvp)
   386                  vput(lvp);
   387
   388          vrele(rvp);
   389
   390          return (error);
   391  }



The following program and command expose the problem:

$ cat /tmp/test.c

#include <err.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int
main(int argc, char *argv[])
{
        char    buf[3];

        if (getcwd(buf, sizeof(buf)) == NULL)
                err(EXIT_FAILURE, "getcwd");

        printf("cwd = \"%s\"\n", buf);

        return EXIT_SUCCESS;
}
$ mkdir /tmp/a && cd /tmp/a
$ pwd
/tmp/a
$ cc /tmp/test.c && ./a.out
cwd = "/a"
$ 

The buffer is large enough to contain the first vnode name ("/a"), but
the vnode limit (sizeof(buf)/2 = 3/2 = 1 vnode) is reached before lvp to
be found. As the limit isn't checked, getcwd(3) returns successfully
with a wrong result (partial path).


The following diff adds check for limit in vfs_getcwd_common() loop.
It returns a ERANGE error, which is propagated to sys___getcwd().

The ERANGE error could already by returned by vfs_getcwd_common() in
some cases (at least by vfs_getcwd_scandir()), so it shouldn't introduce
new error that would need checking by callers.

While here, I added sys___getcwd_args struct description.

Thanks.
-- 
Sebastien Marie

 
Index: kern/vfs_getcwd.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_getcwd.c,v
retrieving revision 1.27
diff -u -p -r1.27 vfs_getcwd.c
--- kern/vfs_getcwd.c   28 Jul 2017 21:54:49 -0000      1.27
+++ kern/vfs_getcwd.c   10 Aug 2017 07:56:23 -0000
@@ -374,6 +374,8 @@ vfs_getcwd_common(struct vnode *lvp, str
 
        } while ((lvp != rvp) && (limit > 0)); 
 
+       if ((limit <= 0) && (lvp != rvp))
+               error = ERANGE;
 out:
 
        if (bpp)
@@ -394,7 +396,10 @@ out:
 int
 sys___getcwd(struct proc *p, void *v, register_t *retval) 
 {
-       struct sys___getcwd_args *uap = v;
+       struct sys___getcwd_args /* {
+               syscallarg(char *) buf;
+               syscallarg(size_t) len;
+       } */    *uap = v;
        int error, lenused, len = SCARG(uap, len);
        char *path, *bp, *bend;
 

Reply via email to