<i18n dev> RFR(xs): 8165936: Potential Heap buffer overflow when seaching timezone info files

[Thomas Stüfe]

Hi all,

thanks for the reviews. Here is version two:

http://cr.openjdk.java.net/~stuefe/webrevs/8165936-Potential-Heap-buffer-overflow-when-seaching-timezone-info-files/webrev.01/webrev/

Only cosmetic changes:
- made code pre-c99 compatible
- consistently use dirent64
- fix indentation in ifs
- removed black between malloc and cast

Kind Regards, Thomas



On Tue, Sep 13, 2016 at 5:25 PM, Masayoshi Okutsu <
masayoshi.okutsu at oracle.com> wrote:

> Looks good to me. Thank you for fixing this bug!
>
> Masayoshi
>
>
>
> On 9/13/2016 11:49 PM, Thomas Stüfe wrote:
>
>> Hi Christoph, thanks for your review! Yes, I can remove the blank.
>>
>> Kind Regards, Thomas
>>
>> On Tue, Sep 13, 2016 at 2:35 PM, Langer, Christoph <
>> christoph.langer at sap.com
>>
>>> wrote:
>>> Hi Thomas,
>>>
>>> your change looks good. I'm also forwarding this to i18n-dev as issues in
>>> TimeZone implementation are mostly handled there.
>>>
>>> One remark: Can you take the opportunity to also remove the blank between
>>> the cast and malloc in line 150: "(struct dirent64 *) malloc..."?
>>>
>>> Unfortunately I'm no reviewer, so you still need an official review.
>>>
>>> Best regards
>>> Christoph
>>>
>>> -----Original Message-----
>>>> From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On
>>>>
>>> Behalf
>>>
>>>> Of Thomas Stüfe
>>>> Sent: Dienstag, 13. September 2016 12:54
>>>> To: Java Core Libs <core-libs-dev at openjdk.java.net>
>>>> Subject: RFR(xs): 8165936: Potential Heap buffer overflow when seaching
>>>> timezone info files
>>>>
>>>> Dear all,
>>>>
>>>> please take a look at this small change:
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8165936
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-
>>>>
>>> Potential-Heap-buffer-
>>>
>>>> overflow-when-seaching-timezone-info-files/webrev.00/webrev/
>>>>
>>>> readdir_r is used to iterate over the content of a system directory, but
>>>> the buffer passed to it is too small: Its size should include the size
>>>> of
>>>> the dirent structure itself (minus the d_name member).
>>>>
>>>> The fix also now checks the return code of pathconf(), and if pathconf()
>>>> returns an error, falls back to the NAME_MAX compile time constant.
>>>> Finally, it imposes a minimum size for the buffer, because on older
>>>>
>>> System
>>>
>>>> V systems NAME_MAX may be surprisingly small and readdir_r will not
>>>> check
>>>> the output buffer size. I think it is better to err on the safe side
>>>>
>>> here.
>>>
>>>> Kind Regards, Thomas
>>>>
>>>
>