[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-i18n-dev
Subject:    Re: <i18n dev> RFR(xs): 8165936: Potential Heap buffer overflow when seaching timezone info files
From:       Roger Riggs <Roger.Riggs () Oracle ! com>
Date:       2016-09-15 17:39:13
Message-ID: 00b6b2a9-e394-18a7-148d-2e82d432a020 () Oracle ! com
[Download RAW message or body]

Hi Thomas,

It looks like NAME_MAX is not defined on Solaris and breaks the build on 
Solaris. [1]

An  alternative would be to conditionally use PATH_MAX.

But I think it would be reasonable to just remove the use of NAME_MAX, 
since the following
code increases the value to at least 1024, which should be safe.

If you don't have time to address this, I can propose a fix.

Roger

[1] 8166148  fix for JDK-8165936 broke solaris builds 
<https://bugs.openjdk.java.net/browse/JDK-8166148>


On 9/14/2016 2:34 AM, Thomas Stüfe wrote:
> 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/ \
>  <http://cr.openjdk.java.net/%7Estuefe/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@oracle.com <mailto:masayoshi.okutsu@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@sap.com <mailto:christoph.langer@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@openjdk.java.net
> <mailto:core-libs-dev-bounces@openjdk.java.net>] On
> 
> Behalf
> 
> Of Thomas Stüfe
> Sent: Dienstag, 13. September 2016 12:54
> To: Java Core Libs <core-libs-dev@openjdk.java.net
> <mailto:core-libs-dev@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
> <https://bugs.openjdk.java.net/browse/JDK-8165936>
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-
> <http://cr.openjdk.java.net/%7Estuefe/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
> 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]