From 0fed5fde65e4a0cd600dc181e5b3c42d1147df51 Mon Sep 17 00:00:00 2001 From: aszlig Date: Fri, 2 Jan 2015 03:27:39 +0100 Subject: [PATCH] libutil: Improve errmsg on readLink size mismatch. A message like "error: reading symbolic link `...' : Success" really is quite confusing, so let's not indicate "success" but rather point out the real issue. We could also limit the check of this to just check for non-negative values, but this would introduce a race condition between stat() and readlink() if the link target changes between those two calls, thus leading to a buffer overflow vulnerability. Reported by @Ericson2314 on IRC. Happened due to a possible ntfs-3g bug where a relative symlink returned the absolute path (st_)size in stat() while readlink() returned the relative size. Signed-off-by: aszlig Tested-by: John Ericson --- nix/libutil/util.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 7998664ed0..410d0f2830 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -193,8 +193,12 @@ Path readLink(const Path & path) if (!S_ISLNK(st.st_mode)) throw Error(format("`%1%' is not a symlink") % path); char buf[st.st_size]; - if (readlink(path.c_str(), buf, st.st_size) != st.st_size) - throw SysError(format("reading symbolic link `%1%'") % path); + ssize_t rlsize = readlink(path.c_str(), buf, st.st_size); + if (rlsize == -1) + throw SysError(format("reading symbolic link '%1%'") % path); + else if (rlsize != st.st_size) + throw Error(format("symbolic link '%1%' size mismatch %2% != %3%") + % path % rlsize % st.st_size); return string(buf, st.st_size); }