Hello, On Sun, 01 May 2011, Sascha Manns wrote:

i've got a bugreport for the boinc-client package: https://bugzilla.novell.com/show_bug.cgi?id=689499.

Maybe anyone can help me with this bug? I have searched the whole Packaging Sources, but doesn't found anything and haven't any idea.

So it would be great, if anyone can help.

Have a nice Rest Weekend. :-) Sascha

==== lib/filesys.cpp:349 ==== int dir_size(const char* dirpath, double& size, bool recurse) { #ifdef WIN32 char path2[_MAX_PATH]; sprintf(path2, "%s/*", dirpath); [.. : 373] #else char filename[256], subdir[256]; int retval=0; DIRREF dirp; double x; size = 0; dirp = dir_open(dirpath); if (!dirp) return ERR_OPENDIR; while (1) { retval = dir_scan(filename, dirp, sizeof(filename)); if (retval) break; sprintf(subdir, "%s/%s", dirpath, filename); ==== dirpath and filename are written to subdir withouth any check, and subdir is a mere 255 bytes long, filename alone can be the same size. Just a crude idea: ==== Top of file: ==== #ifndef _POSIX_SOURCE #define _POSIX_SOURCE 1 #endif #include <limits.h> #include <errno.h> [..: former line 373] #else char filename[NAME_MAX], subdir[PATH_MAX]; [..] if (retval) break; retval = snprintf(subdir, PATH_MAX-2, "%s/%s", dirpath, filename); if(retval >= PATH_MAX-2) { errno = ENAMETOOLONG; perror(""); /* oder so ähnlich */ break; } [..] ==== Someone who knows C better should check that though. AFAIK the limit of PATH_MAX-2 ('/' + '\0' + dirpath (w/o '\0') + filename (w/o '\0')) should be ok, but ... HTH, -dnh -- In /etc is what you think. In /proc is, what the OS thinks. -- Thomas Blum in doc -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org

Hello Cristian, Cristian Rodríguez <crrodriguez@opensuse.org> wrote at Sunday 15 May 2011:

It is a security hole, specifically, a buffer overflow, where do I get the package sources *exactly* to take a look ? Aaha that is a buffer overflow... You can see the package in newtork/boinc-client. You're welcome :-) -- Sincerely Yours

Sascha Manns open-slx Community & Support Agent openSUSE Membership Comitee openSUSE Marketing Team Web: http://saigkill.homelinux.net German Community Portal: http://community.open-slx.de -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org

Hello, On Sun, 15 May 2011, Cristian Rodríguez wrote:

El 10/05/11 15:15, David Haller escribió:

...

==== Top of file: ==== #ifndef _POSIX_SOURCE #define _POSIX_SOURCE 1 #endif #include <limits.h> #include <errno.h> [..: former line 373] #else char filename[NAME_MAX], subdir[PATH_MAX]; [..] if (retval) break; retval = snprintf(subdir, PATH_MAX-2, "%s/%s", dirpath, filename); if(retval >= PATH_MAX-2) { errno = ENAMETOOLONG; perror(""); /* oder so ähnlich */ break; } [..] ====

Someone who knows C better should check that though. AFAIK the limit of PATH_MAX-2 ('/' + '\0' + dirpath (w/o '\0') + filename (w/o '\0')) should be ok, but ...

There is other problems with this stuff that upstream has to fix, see the function definition

int dir_size(const char* dirpath, double& size, bool recurse) .. that's going to fail with an integer overflow sooner or later...

** off_t *** dir_size ... and fix the underlying code that is not going to handle stuff correctly in its currrent incarnation...

I'm not that good a C programmer, especially with such stuff, I can identify the problem at times, but fixing it only in really trivial cases[1]. A test program here even failed even with snprintf (but I was tired when I did that, as I'm now). So, better someone experienced should fix that function (and possibly the Windows part as well). Upstream? Maybe something more like a check before snprintf? if( (size_t)(PATH_MAX-2) <= ( strlen(dirpath) + strlen(filename) ) ) { errno = ENAMETOOLONG; perror(""); break; } retval = snprintf(subdir, PATH_MAX-2, "%s/%s", dirpath, filename); if(retval >= PATH_MAX-2 || retval < 0 ) { errno = ENAMETOOLONG; perror(""); exit(errno); } Remember, with stuff like that, I'm a layman. -dnh [1] e.g. a classic _and_ easily identifiable off-by-one error -- But, as we all know, "robust" and "stable" have different meanings in the computer industry. "robust": Probably won't fall over if a gnat farts nearby (unless the gnat is near a sensitive spot). "stable": no longer updated or supported by the manufacturer. -- Steve VanDevender -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org

* Cristian Rodríguez (crrodriguez@opensuse.org) [20110515 05:56]:

I would just avoid the whole issue by using asprintf() or a library that has a proper call to check the (approximate) size of a directory...

Or the upstream folks really should write C++ code and not C in disguise. Then the handling of filesystem related stuff would be much easier as things like dynamic strings are part of the language. BTW, Boost::Filesystem comes to mind for the filesystem related stuff. Philipp -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org

El 16/05/11 09:16, Philipp Thomas escribió:

* Cristian Rodríguez (crrodriguez@opensuse.org) [20110515 05:56]:

...

I would just avoid the whole issue by using asprintf() or a library that has a proper call to check the (approximate) size of a directory...

Or the upstream folks really should write C++ code and not C in disguise. Then the handling of filesystem related stuff would be much easier as things like dynamic strings are part of the language. BTW, Boost::Filesystem comes to mind for the filesystem related stuff.

Yeah, using boost's recursive_directory_iterator will save the day here. -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org