[jira] Commented: (XERCESC-1319) Buffer overflow in ICULCPTranscoder::transcode

["Alberto Massari (JIRA)" <xe...@xml.apache.org>]

     [ http://nagoya.apache.org/jira/browse/XERCESC-1319?page=comments#action_57089 ]
     
Alberto Massari commented on XERCESC-1319:
------------------------------------------

Hi Alex,
I have some questions regarding your bug report:

1) you say that the API to be fixed is ICULCPTranscoder::transcode(const char* const toTranscode), but that API uses the ICU ucnv_toUChars API, not the ucnv_fromUChars that you name in the report. Did you mean ICULCPTranscoder::transcode(const XMLCh* const toTranscode) ?

2) you say that your fix aims at converting the warning U_STRING_NOT_TERMINATED_WARNING into an error U_BUFFER_OVERFLOW_ERROR so that the buffer is reallocated. But in that function there is a test for both return codes, so it shouldn't make a difference.

3) your bug report seems to address the same issue reported at http://nagoya.apache.org/jira/browse/XERCESC-1300 but the fix you propose is the opposite suggested there. I am inclined to commit bug# 1300; can you verify if that fix is OK also for you?

4) could you provide a simple testcase for the problem? We just need the UTF-16 string that you provide as the argument to the transcode() API.

Thanks,
Alberto

> Buffer overflow in ICULCPTranscoder::transcode
> ----------------------------------------------
>
>          Key: XERCESC-1319
>          URL: http://nagoya.apache.org/jira/browse/XERCESC-1319
>      Project: Xerces-C++
>         Type: Bug
>   Components: Utilities
>  Environment: All Platforms
>     Reporter: Alex R. Herbstritt

>
> I have found a bug in the transcoder code when transcoding from UTF-16 to UTF-8. We use Xerces against an in house library so I cannot include the code that reproduces the bug. But the bug has been reproduced on Windows and HPUX32. Instead I will give the details of the bug - along with the fix.
> The bug is a buffer over run that happens in a very special case. The fix for it is very simple. I find it hard to believe that nobody has seen this bug before.
> The problem is located in the file
> xercesc/util/Transcoders/ICU/ICUTranService.cpp
> in the method
> XMLCh* ICULCPTranscoder::transcode(const char* const toTranscode)
> with the function call ucnv_fromUChars:
> targetCap = ucnv_fromUChars
>         (
>             fConverter
>             , retBuf
>             , targetLen + 1
>             , actualSrc
>             , -1
>             , &err
>         );
> This is the function that is doing the actual conversion. The problem is with the "targetLen + 1" - this should be replaced with "targetLen". (Note that the call that follows has "targetCap", not "targetCap + 1".)
> The problem is that ucnv_fromUChars can fill the buffer up, including the space held for the null term. That is, targetCap is returned equaling targetLen+1, along with a U_STRING_NOT_TERMINATED_WARNING. This is all fine, until the end of the method where,
>     // Cap it off and return
>     retBuf[targetCap] = 0;
>     return retBuf;
> will place the null term outside of the buffer. That is, we should never let targetCap be larger than targetLen. (The buffer overflow will only happen when targetCap==targetLen+1.)
> Replacing "targetLen + 1" with "targetLen" results in a U_BUFFER_OVERFLOW_ERROR. This is correct, because in the overflow case the problem is that the new string created is one byte longer than the buffer that was allocated. So we want the error to cause a new buffer to be allocated.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://nagoya.apache.org/jira/secure/Administrators.jspa
-
If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xerces-c-dev-help@xml.apache.org