Fix several buffer overflows in realpath(3).

- The statement "left_len -= s - left;" does not take the slash into
  account if one was found. This results in the invariant
  "left[left_len] == '\0'" being violated (and possible buffer
  overflows). The patch replaces the variable "s" with a size_t
  "next_token_len" for more clarity.
- "slen" from readlink(2) can be 0 when encountering empty
  symlinks. Then, further down, "symlink[slen - 1]" underflows the
  buffer. When slen == 0, realpath(3) should probably return ENOENT
  (http://austingroupbugs.net/view.php?id=825,
  https://lwn.net/Articles/551224/).

Some other minor issues:
- The condition "resolved_len >= PATH_MAX" cannot be true.
- Similarly, "s - left >= sizeof(next_token)" cannot be true, as long
  as "sizeof(next_token) >= sizeof(left)".
- Return ENAMETOOLONG when a resolved symlink from readlink(2) is too
  long for the symlink buffer (instead of just truncating it).
- "resolved_len > 1" below the call to readlink(2) is always true as
  "strlcat(resolved, next_token, PATH_MAX);" always results in a
  string of length > 1. Also, "resolved[resolved_len - 1] = '\0';" is
  not needed; there can never be a trailing slash here.
- The truncation check for "strlcat(symlink, left, sizeof(symlink));"
  should be against "sizeof(symlink)" (the third argument to strlcat)
  instead of "sizeof(left)".

Submitted by:	Jan Kokemц╪ller <jan.kokemueller@gmail.com>
PR:	219154
MFC after:	2 weeks
This commit is contained in:
Konstantin Belousov 2017-05-15 17:14:53 +00:00
parent 6afab5ccc5
commit f81e5b2d9b

View file

@ -51,10 +51,11 @@ char *
realpath(const char * __restrict path, char * __restrict resolved)
{
struct stat sb;
char *p, *q, *s;
size_t left_len, resolved_len;
char *p, *q;
size_t left_len, resolved_len, next_token_len;
unsigned symlinks;
int m, slen;
int m;
ssize_t slen;
char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
if (path == NULL) {
@ -109,18 +110,19 @@ realpath(const char * __restrict path, char * __restrict resolved)
* and its length.
*/
p = strchr(left, '/');
s = p ? p : left + left_len;
if (s - left >= sizeof(next_token)) {
if (m)
free(resolved);
errno = ENAMETOOLONG;
return (NULL);
next_token_len = p ? p - left : left_len;
memcpy(next_token, left, next_token_len);
next_token[next_token_len] = '\0';
if (p != NULL) {
left_len -= next_token_len + 1;
memmove(left, p + 1, left_len + 1);
} else {
left[0] = '\0';
left_len = 0;
}
memcpy(next_token, left, s - left);
next_token[s - left] = '\0';
left_len -= s - left;
if (p != NULL)
memmove(left, s + 1, left_len + 1);
if (resolved[resolved_len - 1] != '/') {
if (resolved_len + 1 >= PATH_MAX) {
if (m)
@ -173,19 +175,25 @@ realpath(const char * __restrict path, char * __restrict resolved)
errno = ELOOP;
return (NULL);
}
slen = readlink(resolved, symlink, sizeof(symlink) - 1);
if (slen < 0) {
slen = readlink(resolved, symlink, sizeof(symlink));
if (slen <= 0 || slen >= sizeof(symlink)) {
if (m)
free(resolved);
if (slen < 0) {
/* keep errno from readlink(2) call */
} else if (slen == 0) {
errno = ENOENT;
} else {
errno = ENAMETOOLONG;
}
return (NULL);
}
symlink[slen] = '\0';
if (symlink[0] == '/') {
resolved[1] = 0;
resolved_len = 1;
} else if (resolved_len > 1) {
} else {
/* Strip the last path component. */
resolved[resolved_len - 1] = '\0';
q = strrchr(resolved, '/') + 1;
*q = '\0';
resolved_len = q - resolved;
@ -209,7 +217,7 @@ realpath(const char * __restrict path, char * __restrict resolved)
}
left_len = strlcat(symlink, left,
sizeof(symlink));
if (left_len >= sizeof(left)) {
if (left_len >= sizeof(symlink)) {
if (m)
free(resolved);
errno = ENAMETOOLONG;