[filesystem] directory_iterator may cause buffer overflow
Hello, On boost 1.33.0 or later, class directory_iterator is implemented by readdir_r() for some POSIX platform. readdir_r() require a memory buffer which is larger than offsetof(dirent,d_name) + pathconf(dirname,_PC_NAME_MAX) + 1. But in operations.cpp, the size is sizeof(dirent). On some platform such as Linux, FreeBSD and Cygwin, a size of d_name member is 256. This is a enough size in many cases. OTOH, the size on Solaris and BeOS is 1. Therefore directory_iterator always cause buffer overflow. See also: http://thread.gmane.org/gmane.comp.lib.boost.devel/115104 http://womble.decadentplace.org.uk/readdir_r-advisory.html In the article above, calling readdir_r() after pathconf() has race condition, so it is necessary to secure the size that seems to be perhaps enough. Possibly, this problem might cause http://thread.gmane.org/gmane.comp.lib.boost.devel/135820 Regards, Takeshi Mouri
"Takeshi Mouri" <takeshi.mouri.net@green.ocn.ne.jp> wrote in message news:B7C6048A94ECDFtakeshi.mouri.net@green.ocn.ne.jp...
Hello,
On boost 1.33.0 or later, class directory_iterator is implemented by readdir_r() for some POSIX platform. readdir_r() require a memory buffer which is larger than offsetof(dirent,d_name) + pathconf(dirname,_PC_NAME_MAX) + 1. But in operations.cpp, the size is sizeof(dirent).
On some platform such as Linux, FreeBSD and Cygwin, a size of d_name member is 256. This is a enough size in many cases. OTOH, the size on Solaris and BeOS is 1. Therefore directory_iterator always cause buffer overflow.
Ouch! I was misreading the POSIX spec.
See also: http://thread.gmane.org/gmane.comp.lib.boost.devel/115104 http://womble.decadentplace.org.uk/readdir_r-advisory.html
In the article above, calling readdir_r() after pathconf() has race condition, so it is necessary to secure the size that seems to be perhaps enough.
Possibly, this problem might cause http://thread.gmane.org/gmane.comp.lib.boost.devel/135820
The same thought occurred to me. I'll try to get a fix in tomorrow. Thanks for the report, --Beman
"Beman Dawes" <bdawes@acm.org> wrote in message news:do7smm$p9u$1@sea.gmane.org...
"Takeshi Mouri" <takeshi.mouri.net@green.ocn.ne.jp> wrote in message news:B7C6048A94ECDFtakeshi.mouri.net@green.ocn.ne.jp...
Hello,
On boost 1.33.0 or later, class directory_iterator is implemented by readdir_r() for some POSIX platform. readdir_r() require a memory buffer which is larger than offsetof(dirent,d_name) + pathconf(dirname,_PC_NAME_MAX) + 1. But in operations.cpp, the size is sizeof(dirent).
On some platform such as Linux, FreeBSD and Cygwin, a size of d_name member is 256. This is a enough size in many cases. OTOH, the size on Solaris and BeOS is 1. Therefore directory_iterator always cause buffer overflow.
Ouch! I was misreading the POSIX spec.
See also: http://thread.gmane.org/gmane.comp.lib.boost.devel/115104 http://womble.decadentplace.org.uk/readdir_r-advisory.html
In the article above, calling readdir_r() after pathconf() has race condition, so it is necessary to secure the size that seems to be perhaps enough.
Possibly, this problem might cause http://thread.gmane.org/gmane.comp.lib.boost.devel/135820
The same thought occurred to me.
I'll try to get a fix in tomorrow.
Thanks for the report,
CVS head has now been updated. --Beman
See also: http://thread.gmane.org/gmane.comp.lib.boost.devel/115104 http://womble.decadentplace.org.uk/readdir_r-advisory.html
In the article above, calling readdir_r() after pathconf() has race condition, so it is necessary to secure the size that seems to be perhaps enough.
CVS head has now been updated.
--Beman
Sorry, my explanation seems not to have been good.
BOOST_FILESYSTEM_DECL boost::filesystem::system_error_type dir_itr_first( void *& handle, void *& buffer, const std::string & dir, std::string & target, status_flags &, status_flags & ) { static const std::string dummy_first_name( "." ); if ( (handle = ::opendir( dir.c_str() )) == 0 ) return errno; target = dummy_first_name; long pc_name_max( ::pathconf( dir.c_str(), _PC_NAME_MAX ) ); if ( pc_name_max == -1L ) return errno; dirent de; buffer = std::malloc( (sizeof(dirent) - sizeof(de.d_name)) + static_cast<std::size_t>( pc_name_max ) + 1 ); return 0; }
When path 'dir' is a symbolic link, the directories opened by opendir() and pathconf() might be different. This is the race condition that I wanted to say. Though I think that this limitation is acceptable, it is necessary to explain it in the document at least. Another solution is to examine an accurate value by using fpathconf() and the platform-dependent function (ex. dirfd, fdopendir) as much as possible. In the article above, Ben Hutchings recommends this. Regards, Takeshi Mouri
participants (2)
-
Beman Dawes -
Takeshi Mouri