[opensuse-packaging] boinc-client: bnc#689499
Hello Mates, 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 -- 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, 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
El 10/05/11 15:15, David Haller escribió:
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
It is a security hole, specifically, a buffer overflow, where do I get the package sources *exactly* to take a look ?
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 ...
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... -- 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
* 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
* Cristian Rodríguez (crrodriguez@opensuse.org) [20110516 18:03]:
Yeah, using boost's recursive_directory_iterator will save the day here.
Not only that but as far as I checked filesys.cpp you could either turn all functions into wrappers for boost::filesystem or change all callers to directly use boost. In the meantime I've prepared a patch that uses PATH_MAX for the variable filesystem and uses asprintf instead of sprintf. It's attached below. Philipp
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... -- 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) [20110516 04:04]:
int dir_size(const char* dirpath, double& size, bool recurse) .. that's going to fail with an integer overflow sooner or later...
Beg your pardon, but where will it lead to an integer overflow? AFAICT it's simply used to print directory sizes. Though I don't see any reason why you would want to use a double for that and not to suitably sized integer.
** off_t *** dir_size ... and fix the underlying code that is not going to handle stuff correctly in its currrent incarnation...
This remark I don"t understand. Care to expand a bit? Philipp -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
El 16/05/11 08:52, Philipp Thomas escribió:
** off_t *** dir_size ... and fix the underlying code that is not going to handle stuff correctly in its currrent incarnation...
Oh, I read it wrong, discard this wrong analysis, the function does indeed return type int , the counter is the "size" variable ^_^ -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
participants (4)
-
Cristian Rodríguez -
David Haller -
Philipp Thomas -
Sascha Manns