Hello all.
I am trying to wrap up the next upload of ICU and I wanted to get some
additional thoughts on the change for CVE-2014-9911 [0].
The upstream ticket [1] was recently made public and it contains a proof
of concept which I am using to verify my fix. Sadly, the upstream
changes [2] (which are the same as the jessie/stretch/sid changes) do
not apply to ICU in wheezy because in the version of ICU in which the
change was applied uresbund.c has become uresbund.cpp. The fix which
upstream implemented will not work at all in wheezy ICU's uresbund.c. I
tried several different things, but finally settled on the following:
Index: icu-4.8.1.1/source/common/uresbund.c
===================================================================
--- icu-4.8.1.1.orig/source/common/uresbund.c 2016-12-15 21:21:49.561715402 +0000
+++ icu-4.8.1.1/source/common/uresbund.c 2016-12-15 21:43:16.041956495 +0000
@@ -1704,7 +1704,7 @@
char path[256];
char* myPath = path;
const char* resPath = resB->fResPath;
- int32_t len = resB->fResPathLen;
+ int32_t len = uprv_min(resB->fResPathLen, 256);
while(res == RES_BOGUS && dataEntry->fParent != NULL) { /* Otherwise, we'll look in parents */
dataEntry = dataEntry->fParent;
@@ -1712,7 +1712,7 @@
if(dataEntry->fBogus == U_ZERO_ERROR) {
uprv_strncpy(path, resPath, len);
- uprv_strcpy(path+len, inKey);
+ uprv_strncpy(path+len, inKey, 256-len);
myPath = path;
key = inKey;
do {
With that change, the proof of concept no loner causes a crash. I
believe that it accomplishes the same intent as upstream's c++ change,
though in a less elegant/robust fashion.
My question is: do others agree that this is a valid and viable fix for
CVE-2014-9911?
Regards,
-Roberto
[0] https://security-tracker.debian.org/tracker/CVE-2014-9911
[1] http://bugs.icu-project.org/trac/ticket/10891
[2] http://bugs.icu-project.org/trac/changeset/35699
--
Roberto C. Sánchez
http://people.connexer.com/~roberto
http://www.connexer.com
Attachment:
signature.asc
Description: Digital signature