Re: RFC: Handling CVE-2014-9911 in ICU

[Raphael Hertzog <hertzog@debian.org>]

Hi Roberto,

On Thu, 15 Dec 2016, Roberto C. Sánchez wrote:
> @@ -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);

After use of strncpy, you have no guarantee that the string is null
terminated. You should thus add this:
path[255] = '\0'

strncpy() avoids the buffer overwrite but if it's not NULL terminated
you might have another "read outside of buffer" problem later on.

Apart from that, assuming that the underlying problem is the buffer overwrite
with strcpy(), then your fix should be good enough, yes.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/