Boost.Filesystem: basename function is not compatible with POSIX; potential for path-related security issues

Hi, I got the following report [1] for Boost.Filesystem from a Debian user. Before entering into trac, I thought I'd ask whether this deviation from POSIX is by design or is a bug. Thanks, -Steve P.S. The original report is based on Boost 1.40, but I verified the same behaviour on Boost 1.41. [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=565504 ----- Forwarded message from Roger Leigh <rleigh@debian.org> ----- Package: libboost-filesystem1.40.0 Version: 1.40.0-5 Severity: important The basename function is not compatible with the POSIX function by the same name: Path POSIX Boost test.real test.real test /usr/bin/perl perl perl /usr/lib lib lib /usr/ usr usr usr usr / / / . . .. .. . The test program is attached. Just compile with g++ -o testbasename -lboost_filesystem testbasename.cc http://www.opengroup.org/onlinepubs/000095399/functions/basename.html ??? It is not stripping trailing backslashes. ??? "if ph.leaf() contains a dot ('.'), returns the substring of ph.leaf() starting from beginning and ending at the last dot (the dot is not included). Otherwise, returns ph.leaf()". This is wrong, shown by the paths returned for "." ("") and ".." (".") above. The latter could lead to reading and writing using the wrong path, which could have security issues if used in a secure context. This might be justification for raising the severity of this bug. Looking at the API reference, it looks like extension() and basename() may be intended to be complementary and are for splitting a filename into its main part and extension part, *not* the directory and filename components of a path. This should probably be explicitly spelled out due to the dangerous confusion which may result if used inappropriately. In particular, "." and ".." definitely need special casing--these are not extension separators and basename should return them intact; extension() should return an empty string. I noticed this when converting schroot to use the boost convenience function instead of my own. For reference, this is my version: std::string sbuild::basename (std::string name, char separator = '/') { // Remove trailing separators std::string::size_type cur = name.length(); while (cur > 0 && name[cur - 1] == separator) --cur; name.resize(cur); // Find last separator std::string::size_type pos = name.rfind(separator); std::string ret; if (pos == std::string::npos) ret = name; // No separators else if (pos == 0 && name.length() == 1 && name[0] == separator) ret = separator; // Only separators else ret = name.substr(pos + 1); // Basename only // Remove any duplicate adjacent path separators return remove_duplicates(ret, separator); } A POSIX-compatible dirname() function would nicely complement a POSIX-compatible basename() function as an addition to boost::filesystem. It looks like these are orthogonal to the existing functionality, however. Regards, Roger -- System Information: Debian Release: squeeze/sid APT prefers unstable APT policy: (550, 'unstable') Architecture: amd64 (x86_64) Kernel: Linux 2.6.32-trunk-amd64 (SMP w/4 CPU cores) Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Versions of packages libboost-filesystem1.40.0 depends on: ii libboost-system1.40.0 1.40.0-5 Operating system (e.g. diagnostics ii libc6 2.10.2-5 Embedded GNU C Library: Shared lib ii libgcc1 1:4.4.2-9 GCC support library ii libstdc++6 4.4.2-9 The GNU Standard C++ Library v3 libboost-filesystem1.40.0 recommends no packages. libboost-filesystem1.40.0 suggests no packages. -- no debconf information #include <libgen.h> #include <boost/filesystem/convenience.hpp> #include <cstring> #include <iostream> #include <iomanip> int main () { const char *paths[] = { "test.real", "/usr/bin/perl", "/usr/lib", "/usr/", "usr", "/", ".", "..", 0 }; std::cout << std::setw(16) << std::left << "Path" << std::setw(16) << std::left << "POSIX" << std::setw(16) << std::left << "Boost" << '\n'; for (const char **path = &paths[0]; *path != 0; ++path) { char *bpath = strdup(*path); std::cout << std::setw(16) << std::left << *path << std::setw(16) << std::left << basename(bpath) << std::setw(16) << std::left << boost::filesystem::basename(*path) << '\n'; free(bpath); } return 0; } _______________________________________________ pkg-boost-devel mailing list pkg-boost-devel@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-boost-devel ----- End forwarded message -----

On Sat, Jan 16, 2010 at 8:45 PM, Steve M. Robbins <steve@sumost.ca> wrote:
Hi,
I got the following report [1] for Boost.Filesystem from a Debian user. Before entering into trac, I thought I'd ask whether this deviation from POSIX is by design or is a bug.
Thanks, -Steve P.S. The original report is based on Boost 1.40, but I verified the same behaviour on Boost 1.41.
[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=565504
----- Forwarded message from Roger Leigh <rleigh@debian.org> -----
Package: libboost-filesystem1.40.0 Version: 1.40.0-5 Severity: important
The basename function is not compatible with the POSIX function by the same name...
Right. That's part of the reason why boost::filesystem::basename() is deprecated. Use path::filename() if you want the POSIX functionality. Use path::stem() if you want the old boost::filesystem::basename() functionality. Use path::extension() if you want the extension. --Beman
participants (2)
-
Beman Dawes
-
Steve M. Robbins