| Summary: | dev-libs/icu: pkgdata: Overflow | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Dron <dron_2> |
| Component: | Current packages | Assignee: | Arfrever Frehtes Taifersar Arahesis (RETIRED) <arfrever> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | bothie, php-bugs, shiningarcanine, srl, zsojka |
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | https://bugs.icu-project.org/trac/ticket/7680 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Attachments: | build.log of icu-4.2.1. | ||
|
Description
Dron
2009-09-20 18:05:13 UTC
Created attachment 204721 [details]
build.log of icu-4.2.1.
Use CFLAGS="-O3 -march=nocona -pipe". (In reply to comment #2) > Use CFLAGS="-O3 -march=nocona -pipe". I meant CFLAGS="-O2 -march=nocona -pipe". I can CONFIRM the bug to be perfectly VALID. It's not caused by any particular CFLAG (or combination of such) but just by generating a buffer overflow in the program pkgdata. A quick and simple fix (which I would recommend to add to the portage tree immediatelly) would be to --- icu/source/tools/pkgdata/pkgdata.cpp +++ icu/source/tools/pkgdata/pkgdata.cpp @@ -103,1 +103,1 @@ -#define SMALL_BUFFER_MAX_SIZE 512 +#define SMALL_BUFFER_MAX_SIZE 2048 which should give enough room for even very long command lines. The true and clean fix would be to get rid of SMALL_BUFFER_MAX_SIZE (and any other #define like that too) all together, but that would be a job for upstream, because it will require a redesign of (hopfully) internal API. BTW: Just tested the change - now ico can be emerged without any problems. @arfrever: After illegally resolving this bug as invalid while it was perfectly VALID, I leave it up to you to inform upstream and push them to fix their code. Regards, Bodo (In reply to comment #4) There wasn't any proof that this bug is valid. Please report it to upstream. *** Bug 318011 has been marked as a duplicate of this bug. *** (In reply to comment #6) > *** Bug 318011 has been marked as a duplicate of this bug. *** > I was typing a response to Bug 318011 until I received an email saying that it had been marked invalid. Here is what I was going to say: > The problem is at pkgdata.cpp:488: > > ... > #define SMALL_BUFFER_MAX_SIZE 512 > ... > static int runCommand(const char* command, UBool specialHandling) { > char cmd[SMALL_BUFFER_MAX_SIZE]; > ... > } else { > normal_command_mode: > sprintf(cmd, "%s", command); // line 488 > } > ... > > When the supplied command line ("x86_64-pc-linux-gnu-gcc -D_REENTRANT -O2 > -pipe ...") is too long, buffer overflow occurs. > > (I have to say, it surprises me that sprintf/strcat is used everywhere in the > code, without any bounds checking) > The file extension implies that it is written in C++, but the code appears to be entirely written in C. If this was C++ code like the file extension had suggested, I would say that they should have used the string and stringstream classes for this, but since it is C, they probably should use something like this: int strncat_fixed ( char * dest, char * src, size_t size ) { return strncat(dest, src, size - strlen(dest) - 1); } In any case, I have filed a ticket with upstream to inform them of this problem: http://bugs.icu-project.org/trac/ticket/7749 Until they fix it, the file could be patched to increase the small and large buffer sizes by a factor 8, which should workaround this for people until upstream hopefully rewrites their code to not rely on fixed-sized buffers for this kind of data in the first place. By the way, if the rest of ICU is written like this and the code interacts with stuff from the internet, which I imagine it does, we could be looking at a much more profound problem, because using these functions without proper bounds checking in a major system component exposed to the internet is a hacker's dream come true. Richard, It would be helpful to us to add such comments to the upstream bug system so that fixes can be made there. We strive to be a responsive (and responsible) project so that things don't need to be 'hacked around' downstream. We do run static and dynamic analysis especially on the core ICU libraries, this aspect in the tool was missed- thank you for finding it. Steven R. Loomis, ICU for C/C++ Technical Lead (In reply to comment #7) > .... > In any case, I have filed a ticket with upstream to inform them of this > problem: > > http://bugs.icu-project.org/trac/ticket/7749 > > Until they fix it, the file could be patched to increase the small and large > buffer sizes by a factor 8, which should workaround this for people until > upstream hopefully rewrites their code to not rely on fixed-sized buffers for > this kind of data in the first place. > > By the way, if the rest of ICU is written like this and the code interacts with > stuff from the internet, which I imagine it does, we could be looking at a much > more profound problem, because using these functions without proper bounds > checking in a major system component exposed to the internet is a hacker's > dream come true. > It seems that the issue is fixed in 4.5.2 and the latest version in portage is 4.4.1. Is it possible to backport the upstream patch to the older versions of icu? Fixed in dev-libs/icu-4.4.1. |
