Re: RFR(xs): 8165936: Potential Heap buffer overflow when seaching timezone info files

[Roger Riggs]

+1

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 blank between malloc and cast

Kind Regards, Thomas

On Tue, Sep 13, 2016 at 5:25 PM, Masayoshi Okutsu <masayoshi.oku...@oracle.com <mailto:masayoshi.oku...@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.lan...@sap.com <mailto:christoph.lan...@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-boun...@openjdk.java.net
                <mailto:core-libs-dev-boun...@openjdk.java.net>] On

            Behalf

                Of Thomas Stüfe
                Sent: Dienstag, 13. September 2016 12:54
                To: Java Core Libs <core-libs-...@openjdk.java.net
                <mailto:core-libs-...@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