[mpg123-devel] [ mpg123-Bugs-1859413 ] mpg123 says bufferoverflow if full path is > 49 characters
Brought to you by:
sobukus
Menu
▾
▴
[mpg123-devel] [ mpg123-Bugs-1859413 ] mpg123 says bufferoverflow if full path is > 49 characters
|
From: SourceForge.net <no...@so...> - 2011-04-08 08:20:48
|
Bugs item #1859413, was opened at 2007-12-27 22:19 Message generated for change (Settings changed) made by sobukus You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=733194&aid=1859413&group_id=135704 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: mpg123 Group: 1.0.0 Status: Closed Resolution: Fixed Priority: 9 Private: No Submitted By: imre_herceg (imreherceg) Assigned to: Nobody/Anonymous (nobody) Summary: mpg123 says bufferoverflow if full path is > 49 characters Initial Comment: I am using openSUSE 10.3 with ALSA output. Since mpg123 1.0rc1 I encounter a problem. When I start the program from the command line, I get the following error message: *** buffer overflow detected ***: mpg123 terminated ======= Backtrace: ========= /lib/libc.so.6(__chk_fail+0x41)[0xb7ddf3b1] /lib/libc.so.6[0xb7ddfaf4] mpg123[0x80572c4] mpg123(open_module+0x11)[0x8057351] mpg123(open_output_module+0x4e)[0x804cafe] mpg123(init_output+0x44)[0x804cc04] mpg123(main+0x2f2)[0x8052d02] /lib/libc.so.6(__libc_start_main+0xe0)[0xb7d1ffe0] mpg123[0x804bfe1] ======= Memory map: ======== 08048000-0805d000 r-xp 00000000 08:02 1642653 /usr/bin/mpg123 0805d000-0805f000 rw-p 00014000 08:02 1642653 /usr/bin/mpg123 0805f000-08080000 rw-p 0805f000 00:00 0 [heap] b7cfc000-b7d06000 r-xp 00000000 08:02 1510365 /lib/libgcc_s.so.1 b7d06000-b7d08000 rw-p 00009000 08:02 1510365 /lib/libgcc_s.so.1 b7d08000-b7d0a000 rw-p b7d08000 00:00 0 b7d0a000-b7e37000 r-xp 00000000 08:02 1510284 /lib/libc-2.6.1.so b7e37000-b7e38000 r--p 0012c000 08:02 1510284 /lib/libc-2.6.1.so b7e38000-b7e3a000 rw-p 0012d000 08:02 1510284 /lib/libc-2.6.1.so b7e3a000-b7e3d000 rw-p b7e3a000 00:00 0 b7e3d000-b7e60000 r-xp 00000000 08:02 1510292 /lib/libm-2.6.1.so b7e60000-b7e62000 rw-p 00022000 08:02 1510292 /lib/libm-2.6.1.so b7e62000-b7e7b000 r-xp 00000000 08:02 1774048 /usr/lib/libmpg123.so.0.0.0 b7e7b000-b7ebe000 rw-p 00019000 08:02 1774048 /usr/lib/libmpg123.so.0.0.0 b7ebe000-b7ece000 rw-p b7ebe000 00:00 0 b7ece000-b7ed0000 r-xp 00000000 08:02 1510290 /lib/libdl-2.6.1.so b7ed0000-b7ed2000 rw-p 00001000 08:02 1510290 /lib/libdl-2.6.1.so b7ed2000-b7ed8000 r-xp 00000000 08:02 2482311 /usr/lib/libltdl.so.3.1.5 b7ed8000-b7eda000 rw-p 00005000 08:02 2482311 /usr/lib/libltdl.so.3.1.5 b7ef7000-b7ef8000 rw-p b7ef7000 00:00 0 b7ef8000-b7f12000 r-xp 00000000 08:02 1512136 /lib/ld-2.6.1.so b7f12000-b7f14000 rw-p 0001a000 08:02 1512136 /lib/ld-2.6.1.so bfa50000-bfa64000 rwxp bfa50000 00:00 0 [stack] bfa64000-bfa65000 rw-p bfa64000 00:00 0 ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso] The strange thing is, if I click on the mp3 file in Konqueror, (mp3 files are associated with mpg123), it opens a Konsole window, and the file is played back. If I make the full path shorter, 49 characters, then playback from the command line is also OK. In one case the full path had to be 49 characters (I counted space and / as 1 character), with an other directory the problem occurred at 46 characters (this however contained 3 accented characters so this also may be 49 characters in UTF8). I checked with mpg123 version 0.68, there I didn't encounter this problem with the same directories. ---------------------------------------------------------------------- Comment By: Thomas Orgis (sobukus) Date: 2007-12-28 17:01 Message: Logged In: YES user_id=470743 Originator: NO Thanks to both of you for the work on this one, closing with new release. ---------------------------------------------------------------------- Comment By: imre_herceg (imreherceg) Date: 2007-12-28 16:47 Message: Logged In: YES user_id=1968536 Originator: YES I also added the line buf=buf2; to the file module.c and compiled the program, now it works allright. Thanks ---------------------------------------------------------------------- Comment By: Thomas Orgis (sobukus) Date: 2007-12-28 16:13 Message: Logged In: YES user_id=470743 Originator: NO Yeah, your code is POSIX.1-2001, which is a portability standard. But we are a bit notorious with mpg123 to keep it working on pre-POSIX systems, even. Call it a spleen;-) Well, or think about MinG"32 platform, which Though I don't suppose the really old systems to use the new module loading stuff (we still have a plain Makefile in place for systems where the autoconf stuff doesn't work...), relying on just malloc, realloc and getcwd is even more portable. Anyhow, going to release the bugfix version 1.0.1 quickly to have that nasty bug squashed. Thanks for the quick dialog! ---------------------------------------------------------------------- Comment By: swi (samuraiwithin) Date: 2007-12-28 15:53 Message: Logged In: YES user_id=1635116 Originator: NO Thanks for quick response. Well, my patch in fact riped from "Advanced Programming in UNIX environment: Second Edition" book :) It's use standard PATH_MAX, that is defined on allmost systems. Rebuild package mpg123 with your patch successfull solve this problem too :) ---------------------------------------------------------------------- Comment By: Thomas Orgis (sobukus) Date: 2007-12-28 15:02 Message: Logged In: YES user_id=470743 Originator: NO Aha, that makes more sense: We are in a directory with >=50 chars path length and call mpg123 with output modules. Konqueror gives full path to mpg123, while the run from command line was from within the file's directory with long path. Long story short: Samuraiwithin, I can fix it with a much smaller patch -- that could mean that I'm smart but I'm the one who failed to add the crucial line in the first place... The idea of the function is right as such: #define PATH_STEP 50 static char *get_the_cwd() { size_t bs = PATH_STEP; char *buf = malloc(bs); while((buf != NULL) && getcwd(buf, bs) == NULL) { char *buf2; buf2 = realloc(buf, bs+=PATH_STEP); if(buf2 == NULL){ free(buf); buf = NULL; } else debug1("pwd: increased buffer to %lu", (unsigned long)bs); } return buf; } But it fails to actually use the reallocated buffer! Please test the following patch, which shall very quickly result into release 1.0.1: Index: src/module.c =================================================================== --- src/module.c (Revision 1312) +++ src/module.c (Arbeitskopie) @@ -157,6 +157,8 @@ buf2 = realloc(buf, bs+=PATH_STEP); if(buf2 == NULL){ free(buf); buf = NULL; } else debug1("pwd: increased buffer to %lu", (unsigned long)bs); + + buf = buf2; } return buf; } My glibc is not complaining about the overflow and it silently works for me (perhaps not for 2000 char paths)... so I cannot definetely confirm the fix but it looks rather obvious. ---------------------------------------------------------------------- Comment By: swi (samuraiwithin) Date: 2007-12-28 13:56 Message: Logged In: YES user_id=1635116 Originator: NO I think this patch solve this problem. At least it's work for me. ================ patch begin =============== diff --git a/mpg123/src/module.c b/mpg123/src/module.c index 25f91e4..3efc7e7 100644 --- a/mpg123/src/module.c +++ b/mpg123/src/module.c @@ -9,6 +9,7 @@ #include <dirent.h> #include <sys/stat.h> #include <unistd.h> +#include <limits.h> #include <errno.h> #include <ctype.h> #include <ltdl.h> @@ -146,19 +147,58 @@ void close_module( mpg123_module_t* module ) } -#define PATH_STEP 50 +#ifdef PATH_MAX +static int pathmax = PATH_MAX; +#else +static int pathmax = 0; +#endif + +static long posix_version = 0; + +#define PATH_MAX_GUESS 1024 +//#define PATH_STEP 50 static char *get_the_cwd() { - size_t bs = PATH_STEP; - char *buf = malloc(bs); - while((buf != NULL) && getcwd(buf, bs) == NULL) + char *buf; + int size; + + if (posix_version == 0) + posix_version = sysconf(_SC_VERSION); + + if (pathmax == 0) + { /* first call of the function */ + errno = 0; + if ((pathmax = pathconf("/", _PC_PATH_MAX)) < 0 ) + { + if (errno == 0) + pathmax = PATH_MAX_GUESS; + else + { + printf("Error call sysconf with _PC_PATH_MAX\n"); + exit(133); + } + } + else + { + pathmax++; + } + } + + size = pathmax; + + if ((buf = malloc(size)) == NULL) { - char *buf2; - buf2 = realloc(buf, bs+=PATH_STEP); - if(buf2 == NULL){ free(buf); buf = NULL; } - else debug1("pwd: increased buffer to %lu", (unsigned long)bs); + printf("Error call malloc\n"); + exit(133); } - return buf; + + if (getcwd(buf,size) == NULL) + { + printf("Error getcwd\n"); + exit(133); + } + + return(buf); } void list_modules() ================ patch end =============== ---------------------------------------------------------------------- Comment By: swi (samuraiwithin) Date: 2007-12-28 13:44 Message: Logged In: YES user_id=1635116 Originator: NO I confirm that bug. morsov@alexey Music/Pink Floyd/Pink Floyd - 1975 - Wish You Were Here $ mpg123 -C Pink\ Floyd\ -\ 04\ -\ Wish\ You\ Were\ Here.mp3 [getlopt.c:115] debug: getsingleopt: -C [getlopt.c:71] debug: int at 0x61cca4 [getlopt.c:77] debug: casting assignment done [getlopt.c:115] debug: getsingleopt: Pink Floyd - 04 - Wish You Were Here.mp3 [module.c:159] debug: pwd: increased buffer to 100 *** buffer overflow detected ***: mpg123 terminated ======= Backtrace: ========= /lib64/libc.so.6(__chk_fail+0x2f)[0x2b66326554ff] /lib64/libc.so.6[0x2b6632655b27] mpg123[0x413f40] mpg123(open_module+0x2f)[0x41400f] mpg123(open_output_module+0x151)[0x405c61] mpg123(init_output+0x4c)[0x405dfc] mpg123(main+0x406)[0x40e636] /lib64/libc.so.6(__libc_start_main+0xf4)[0x2b66325a6c14] mpg123[0x404c99] ======= Memory map: ======== Sure, if i pass fullpath to filename it'll play. So crash appear when get current dir fullpath name. mpg123 expand buffer from initial 50 (PATH_STEP) to 100, but not successfull. ---------------------------------------------------------------------- Comment By: Thomas Orgis (sobukus) Date: 2007-12-28 04:29 Message: Logged In: YES user_id=470743 Originator: NO Hm, cannot confirm that with current version, a command line like that works for me: mpg123 -o alsa -C '/data/thorma/var/music/sampler/1991_techno_attack_original_german_techno-house/06-ministers_of_sex_@_secrets_of_sexual_attraction_(nympho_mix).mp3' The mp3 file path is well above 49 chars. That's on a Source Mage install with glibc 2.4, alsa output would be defalult anyway. But, reading your backtrace /lib/libc.so.6(__chk_fail+0x41)[0xb7ddf3b1] /lib/libc.so.6[0xb7ddfaf4] mpg123[0x80572c4] mpg123(open_module+0x11)[0x8057351] it looks like a crash in the module loader (which is a bit strange if triggered by mp3 file path). Can you compile mpg123 with the -g compiler switch and try the same again? The backtrace (mpg123 ran from the source directory) should look more helpful then. Also, does the crash happen when compiling without dynamic modules (./configure --disable-modules)? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=733194&aid=1859413&group_id=135704 |
