freetype-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Git][freetype/freetype][master] 2 commits: [base] Reject combinations o


From: Werner Lemberg (@wl)
Subject: [Git][freetype/freetype][master] 2 commits: [base] Reject combinations of incompatible `FT_OPEN_XXX` flags.
Date: Tue, 13 Jul 2021 09:03:14 +0000

Werner Lemberg pushed to branch master at FreeType / FreeType

Commits:

3 changed files:

Changes:

  • ChangeLog
    1
    +2021-07-13  Oleg Oshmyan  <chortos@inbox.lv>
    
    2
    +
    
    3
    +	[base] Fix `FT_Open_Face`'s handling of user-supplied streams.
    
    4
    +
    
    5
    +	This was already true (though undocumented) most of the time, but
    
    6
    +	not if `FT_NEW` inside `FT_Stream_New` failed or if the
    
    7
    +	`FT_OPEN_XXX` flags were bad.
    
    8
    +
    
    9
    +	Normally, `FT_Open_Face` calls `FT_Stream_New`, which returns the
    
    10
    +	user-supplied stream unchanged, and in case of any subsequent error
    
    11
    +	in `FT_Open_Face`, the stream is closed via `FT_Stream_Free`.
    
    12
    +
    
    13
    +	Up to now, however, `FT_Stream_New` allocates a new stream even if
    
    14
    +	it is already given one by the user.  If this allocation fails, the
    
    15
    +	user-supplied stream is not returned to `FT_Open_Face` and never
    
    16
    +	closed.  Moreover, the user cannot detect this situation: all they
    
    17
    +	see is that `FT_Open_Face` returns `FT_Err_Out_Of_Memory`, but that
    
    18
    +	can also happen after a different allocation fails within the main
    
    19
    +	body of `FT_Open_Face`, when the user's stream has already been
    
    20
    +	closed by `FT_Open_Face`.  It is plausible that the user stream's
    
    21
    +	`close` method frees memory allocated for the stream object itself,
    
    22
    +	so the user cannot defensively free it upon `FT_Open_Face` failure
    
    23
    +	lest it ends up doubly freed.  All in all, this ends up leaking the
    
    24
    +	memory/resources used by user's stream.
    
    25
    +
    
    26
    +	Furthermore, `FT_Stream_New` simply returns an error if the
    
    27
    +	`FT_OPEN_XXX` flags are unsupported, which can mean either an
    
    28
    +	invalid combination of flags or a perfectly innocent
    
    29
    +	`FT_OPEN_STREAM` on a FreeType build that lacks stream support.
    
    30
    +	With this patch, the user-supplied stream is closed even in these
    
    31
    +	cases, so the user can be sure that if `FT_Open_Face` failed, the
    
    32
    +	stream is definitely closed.
    
    33
    +
    
    34
    +	* src/base/ftobjs.c (FT_Stream_New): Don't allocate a buffer
    
    35
    +	unnecessarily.
    
    36
    +	Move error-handling code to make the control flow more obvious.
    
    37
    +	Close user-supplied stream if the flags are unsupported.
    
    38
    +	`FT_Stream_Open` always sets `pathname.pointer`, so remove the
    
    39
    +	redundant (re)assignment.  None of the `FT_Stream_Open...` functions
    
    40
    +	uses `stream->memory`, so keep just one assignment at the end,
    
    41
    +	shared among all possible control flow paths.
    
    42
    +	('Unsupported flags' that may need a stream closure can be either an
    
    43
    +	invalid combination of multiple `FT_OPEN_XXX` mode flags or a clean
    
    44
    +	`FT_OPEN_STREAM` flag on a FreeType build that lacks stream
    
    45
    +	support.)
    
    46
    +
    
    47
    +2021-07-13  Oleg Oshmyan  <chortos@inbox.lv>
    
    48
    +
    
    49
    +	[base] Reject combinations of incompatible `FT_OPEN_XXX` flags.
    
    50
    +
    
    51
    +	The three modes are mutually exclusive, and the documentation of the
    
    52
    +	`FT_OPEN_XXX` constants notes this.  However, there was no check to
    
    53
    +	validate this in the code, and the documentation on `FT_Open_Args`
    
    54
    +	claimed that the corresponding bits were checked in a well-defined
    
    55
    +	order, implying it was valid (if useless) to specify more than one.
    
    56
    +	Ironically, this documented order did not agree with the actual
    
    57
    +	code, so it could not be relied upon; hopefully, nobody did this and
    
    58
    +	nobody will be hurt by the new validation.
    
    59
    +
    
    60
    +	Even if multiple mode bits were allowed, they could cause memory
    
    61
    +	leaks: if both `FT_OPEN_STREAM` and `stream` are set along with
    
    62
    +	either `FT_OPEN_MEMORY` or `FT_OPEN_PATHNAME`, then `FT_Stream_New`
    
    63
    +	allocated a new stream but `FT_Open_Face` marked it as an 'external'
    
    64
    +	stream, so the stream object was never released.
    
    65
    +
    
    66
    +	* src/base/ftobjs.c (FT_Stream_New): Reject incompatible
    
    67
    +	`FT_OPEN_XXX` flags.
    
    68
    +
    
    1 69
     2021-07-12  Alex Richardson  <Alexander.Richardson@cl.cam.ac.uk>
    
    2 70
     
    
    3 71
     	* meson.build: Fix build for other UNIX systems (e.g., FreeBSD).
    

  • include/freetype/freetype.h
    ... ... @@ -2113,8 +2113,7 @@ FT_BEGIN_HEADER
    2113 2113
        *     Extra parameters passed to the font driver when opening a new face.
    
    2114 2114
        *
    
    2115 2115
        * @note:
    
    2116
    -   *   The stream type is determined by the contents of `flags` that are
    
    2117
    -   *   tested in the following order by @FT_Open_Face:
    
    2116
    +   *   The stream type is determined by the contents of `flags`:
    
    2118 2117
        *
    
    2119 2118
        *   If the @FT_OPEN_MEMORY bit is set, assume that this is a memory file
    
    2120 2119
        *   of `memory_size` bytes, located at `memory_address`.  The data are not
    
    ... ... @@ -2127,6 +2126,9 @@ FT_BEGIN_HEADER
    2127 2126
        *   Otherwise, if the @FT_OPEN_PATHNAME bit is set, assume that this is a
    
    2128 2127
        *   normal file and use `pathname` to open it.
    
    2129 2128
        *
    
    2129
    +   *   If none of the above bits are set or if multiple are set at the same
    
    2130
    +   *   time, the flags are invalid and @FT_Open_Face fails.
    
    2131
    +   *
    
    2130 2132
        *   If the @FT_OPEN_DRIVER bit is set, @FT_Open_Face only tries to open
    
    2131 2133
        *   the file with the driver whose handler is in `driver`.
    
    2132 2134
        *
    
    ... ... @@ -2299,6 +2301,10 @@ FT_BEGIN_HEADER
    2299 2301
        *   See the discussion of reference counters in the description of
    
    2300 2302
        *   @FT_Reference_Face.
    
    2301 2303
        *
    
    2304
    +   *   If `FT_OPEN_STREAM` is set in `args->flags`, the stream in
    
    2305
    +   *   `args->stream` is automatically closed before this function returns
    
    2306
    +   *   any error (including `FT_Err_Invalid_Argument`).
    
    2307
    +   *
    
    2302 2308
        * @example:
    
    2303 2309
        *   To loop over all faces, use code similar to the following snippet
    
    2304 2310
        *   (omitting the error handling).
    

  • src/base/ftobjs.c
    ... ... @@ -197,6 +197,7 @@
    197 197
         FT_Error   error;
    
    198 198
         FT_Memory  memory;
    
    199 199
         FT_Stream  stream = NULL;
    
    200
    +    FT_UInt    mode;
    
    200 201
     
    
    201 202
     
    
    202 203
         *astream = NULL;
    
    ... ... @@ -208,15 +209,15 @@
    208 209
           return FT_THROW( Invalid_Argument );
    
    209 210
     
    
    210 211
         memory = library->memory;
    
    212
    +    mode   = args->flags &
    
    213
    +               ( FT_OPEN_MEMORY | FT_OPEN_STREAM | FT_OPEN_PATHNAME );
    
    211 214
     
    
    212
    -    if ( FT_NEW( stream ) )
    
    213
    -      goto Exit;
    
    214
    -
    
    215
    -    stream->memory = memory;
    
    216
    -
    
    217
    -    if ( args->flags & FT_OPEN_MEMORY )
    
    215
    +    if ( mode == FT_OPEN_MEMORY )
    
    218 216
         {
    
    219 217
           /* create a memory-based stream */
    
    218
    +      if ( FT_NEW( stream ) )
    
    219
    +        goto Exit;
    
    220
    +
    
    220 221
           FT_Stream_OpenMemory( stream,
    
    221 222
                                 (const FT_Byte*)args->memory_base,
    
    222 223
                                 (FT_ULong)args->memory_size );
    
    ... ... @@ -224,33 +225,40 @@
    224 225
     
    
    225 226
     #ifndef FT_CONFIG_OPTION_DISABLE_STREAM_SUPPORT
    
    226 227
     
    
    227
    -    else if ( args->flags & FT_OPEN_PATHNAME )
    
    228
    +    else if ( mode == FT_OPEN_PATHNAME )
    
    228 229
         {
    
    229 230
           /* create a normal system stream */
    
    231
    +      if ( FT_NEW( stream ) )
    
    232
    +        goto Exit;
    
    233
    +
    
    230 234
           error = FT_Stream_Open( stream, args->pathname );
    
    231
    -      stream->pathname.pointer = args->pathname;
    
    235
    +      if ( error )
    
    236
    +        FT_FREE( stream );
    
    232 237
         }
    
    233
    -    else if ( ( args->flags & FT_OPEN_STREAM ) && args->stream )
    
    238
    +    else if ( ( mode == FT_OPEN_STREAM ) && args->stream )
    
    234 239
         {
    
    235 240
           /* use an existing, user-provided stream */
    
    236 241
     
    
    237 242
           /* in this case, we do not need to allocate a new stream object */
    
    238 243
           /* since the caller is responsible for closing it himself       */
    
    239
    -      FT_FREE( stream );
    
    240 244
           stream = args->stream;
    
    245
    +      error  = FT_Err_Ok;
    
    241 246
         }
    
    242 247
     
    
    243 248
     #endif
    
    244 249
     
    
    245 250
         else
    
    251
    +    {
    
    246 252
           error = FT_THROW( Invalid_Argument );
    
    253
    +      if ( ( args->flags & FT_OPEN_STREAM ) && args->stream )
    
    254
    +        FT_Stream_Close( args->stream );
    
    255
    +    }
    
    247 256
     
    
    248
    -    if ( error )
    
    249
    -      FT_FREE( stream );
    
    250
    -    else
    
    251
    -      stream->memory = memory;  /* just to be certain */
    
    252
    -
    
    253
    -    *astream = stream;
    
    257
    +    if ( !error )
    
    258
    +    {
    
    259
    +      stream->memory = memory;
    
    260
    +      *astream       = stream;
    
    261
    +    }
    
    254 262
     
    
    255 263
       Exit:
    
    256 264
         return error;
    


  • reply via email to

    [Prev in Thread] Current Thread [Next in Thread]