Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#107 STLport 5.2.1 basic_filebuf::showmanyc() integer overflow

open-rejected
3
2010-04-29
2010-04-26
Jan Echternach
No

There's a possible integer overflow on 32-bit systems with 64-bit I/O support in STLport if the size of a file is at least 2 GiB.

Discussion

  • Not required: see requirements to relations between streamsize and streamoff (std 2003, 27.4.3.2). It's clear that max positive value for type streamsize is not less then max positive value of type streamoff,

     
    • priority: 5 --> 3
    • assigned_to: nobody --> complement
    • status: open --> open-rejected
     
  • Jan Echternach
    Jan Echternach
    2010-04-29

    It's the other way round: The maximum value of streamsize must fit in streamoff to meet the requirement that streamsize(streamoff(sz))==sz for any streamsize sz.

    For example, gcc 3.4 and above uses a 64-bit integer for streamoff, and ptrdiff_t for streamsize. The current C++0x draft gives as an example streamoff=long long and streamsize="signed size_t". This matches STLports current implementation: On 32-bit platforms with 64-bit I/O support, streamoff=off64_t and streamsize=<some 32-bit signed integer type>. The Sun C++ compiler did warn me that there was a 64-bit integer converted to a 32-bit one, which made me aware of this integer overflow.

     
  • > The maximum value of streamsize must fit in
    > streamoff to meet the requirement that streamsize(streamoff(sz))==sz for
    > any streamsize sz.

    Ill-formed statement (BTW, it removed from current draft), because assume that streamsize (expected unsigned, from common sense) narrow than streamoff (signed, of cause).

    > ... gcc 3.4 and above uses a 64-bit integer for streamoff ...

    This come from libc and kernel, not from compiler (at least directly).
    C++ streams shouldn't conflict with fseek and friends.

    May be just explicit cast enough here.

     
  • Jan Echternach
    Jan Echternach
    2010-04-29

    streamsize must be signed (ISO 14882:2003 27.4.1 "Types") because it's used in one place for negative values, but even the standard agrees that an unsigned type would have been the natural choice otherwise. Anyway, streamsize is required to be signed (27.4.1: "The type streamsize is a synonym for one of the signed basic integral types.").

    The streamsize(streamoff(sz))==sz requirement is still in N3092 27.5.3.2 "fpos requirements". The footnotes with the streamsize and streamoff examples are in N3092 27.5.1 "Types", footnotes #301 and #302.

    >> ... gcc 3.4 and above uses a 64-bit integer for streamoff ...
    >
    > This come from libc and kernel, not from compiler (at least directly).
    > C++ streams shouldn't conflict with fseek and friends.

    No, this is libstdc++'s own decision. gcc 3.3 has typdef long streamoff, newer gcc has typedef long long streamoff. Both libc and kernel support a 32-bit and a 64-bit I/O interface at the same time on every current 32-bit Unix system I know (not just Linux).

    But let's not lose sight of the main point which is that STLport uses off64_t for streamoff if available, while STLport's streamsize remains 32 bits on 32-bit platforms. It is in this constellation that a large file size can be truncated.

     
  • Thanks for point in draft.

    > No, this is libstdc++'s own decision. gcc 3.3 has typdef long streamoff,

    Well, streamoff/streamsize are just reflection of off_t/size_t. And, IMO, should follow ones. BTW, using signed for file size will lead to potential problem: UDF, QFS, ZFS, Btrfs support up to 16EiB files. So 'ssize_t for streamsize' looks not good enough. If for streamoff/off_t/... we may say 'sorry, it's not a seekable stream', then for streamsize/[s]size_t we can't work with such objects at all.

    > But let's not lose sight of the main point which is that STLport uses
    > off64_t for streamoff if available, while STLport's streamsize remains 32
    > bits on 32-bit platforms. It is in this constellation that a large file
    > size can be truncated.

    No. It depends what interface you use. With LARGEFILE_* support you may use large files (if system allow it, of cause). Definition of off_t is depends upon interface (32/64) you use:

    # ifndef __USE_FILE_OFFSET64
    typedef __off_t off_t;
    # else
    typedef __off64_t off_t;
    # endif

    Same redefines for fopen/fopen64, ...

     
  • Jan Echternach
    Jan Echternach
    2010-04-29

    > Well, streamoff/streamsize are just reflection of off_t/size_t.

    For a simple C++ implementation, yes. STLport defines them to match the types of the underlying I/O system, and that's fine with me. I don't want STLport to change its streamoff or streamsize definitions.

    Other implementations are possible. For example, GCC always defines streamoff as a 64-bit type nowadays, even on systems which only support 32-bit file sizes. libstdc++ performs integer type conversions with proper range checks in that case.

    > So 'ssize_t for streamsize' looks not good enough.

    Why? A streamsize is not supposed to represent a file size. It only needs to be large enough to represent the size of a memory buffer. streamoff is for external sizes (i.e. file sizes and offsets), streamsize for internal sizes (I/O buffer sizes in memory).

    Large file I/O on a 32-bit system always means that only small parts of a large file can be in memory at any given point in time, no matter how large the size types in a programming language are. A ptrdiff_t or ssize_t for streamsize ought to be sufficient. However, showmanyc() needs some range checks in this case as it converts an external to an internal size, which is why I made this patch ... :-)

    Defining streamsize to be as large as streamoff only looks like a valid alternative until you notice that this causes 64-bit to 32-bit integer conversions all over the place, requiring an equal amount of integer overflow checks. Overall, distinguishing between external and internal sizes like C++ does and only converting them in a few I/O functions is a lot simpler.

     
  • > Why? A streamsize is not supposed to represent a file size. It only
    > needs to be large enough to represent the size of a memory buffer.
    > streamoff is for external sizes (i.e. file sizes and offsets), streamsize
    > for internal sizes (I/O buffer sizes in memory).

    It's not very clear, thanks to 'fpos requirements':
    streamsize(o)
    streamsize(O(sz)) == sz

    But looks you right: in this context sz is not greater then max positive offset.

    Notes, if somebody else will read this.

    For position in stream (that must represent value as large as max file size) use fpos/fpos_t. In C positioning within stream represented by fgetpos/fsetpos (fpos_t) or ftello/fseeko/lseek (off_t). Use it in preference to ftell/fseek (long).