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

Subject: [boost] Boost.Filesystem: basename function is not compatible with POSIX; potential for path-related security issues
From: Steve M. Robbins (steve_at_[hidden])
Date: 2010-01-16 20:45:15

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_at_[hidden]> -----

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_at_[hidden]
http://lists.alioth.debian.org/mailman/listinfo/pkg-boost-devel

----- End forwarded message -----