Re: [ft-devel] Patch: fix uninitialized variable

[mpsuzuki]

Hi,

Considering the problem you reported, now I think
making FT_Stream_New() initialize *astream always
can be more generic solution.

FT_Stream_New() is NOT public function, but it is
most fundamental function to create a stream in
FreeType2. I suppose no FT2 users write their
function creating FT_Stream without FT_Stream_New(),
so, if FT_Stream_New() initializes *astream always,
it will prevent the bug by freeing uninitialized
stream in their functions.

Following is the revised patch - FT_Stream_New()
initializes *astream. Also public FT_Open_Face()
and private load_face_in_embedded_rfork initialize
their stream variables.

How do you think of?

Regards,
mpsuzuki


Index: ChangeLog
===================================================================
RCS file: /sources/freetype/freetype2/ChangeLog,v
retrieving revision 1.1780
diff -u -r1.1780 ChangeLog
--- ChangeLog   18 Aug 2008 06:02:06 -0000      1.1780
+++ ChangeLog   19 Aug 2008 03:26:55 -0000
@@ -1,3 +1,13 @@
+2008-08-19  suzuki toshiya <address@hidden>
+
+       * src/base/ftobjs.c (FT_Stream_New): Initialize *astream
+       always, even if passed library or arguments are invalid.
+       This fixes a bug that uninitialized stream is freed when
+       an invalid library handle is passed. Originally proposed
+       by Mike Fabian, 2008/08/18 on freetype-devel.
+       (FT_Open_Face): Ditto.
+       (load_face_in_embedded_rfork): Ditto.
+
 2008-08-18  suzuki toshiya <address@hidden>
 
        * src/base/ftmac.c: Add a fallback to suppose the availability
Index: src/base/ftobjs.c
===================================================================
RCS file: /sources/freetype/freetype2/src/base/ftobjs.c,v
retrieving revision 1.291
diff -u -r1.291 ftobjs.c
--- src/base/ftobjs.c   10 Jun 2008 04:57:57 -0000      1.291
+++ src/base/ftobjs.c   19 Aug 2008 03:26:55 -0000
@@ -128,13 +128,14 @@
     FT_Stream  stream;
 
 
+    *astream = 0;
+
     if ( !library )
       return FT_Err_Invalid_Library_Handle;
 
     if ( !args )
       return FT_Err_Invalid_Argument;
 
-    *astream = 0;
     memory   = library->memory;
 
     if ( FT_NEW( stream ) )
@@ -1600,7 +1601,7 @@
     FT_Error   errors[FT_RACCESS_N_RULES];
 
     FT_Open_Args  args2;
-    FT_Stream     stream2;
+    FT_Stream     stream2 = 0;
 
 
     FT_Raccess_Guess( library, stream,
@@ -1713,7 +1714,7 @@
     FT_Error     error;
     FT_Driver    driver;
     FT_Memory    memory;
-    FT_Stream    stream;
+    FT_Stream    stream = 0;
     FT_Face      face = 0;
     FT_ListNode  node = 0;
     FT_Bool      external_stream;




On Tue, 19 Aug 2008 01:47:20 +0900
address@hidden wrote:

>Hi,
>
>On Mon, 18 Aug 2008 18:01:17 +0200
>Mike FABIAN <address@hidden> wrote:
>
>>    error = FT_Stream_New( library, args, &stream );
>
>>      FT_Stream_Free( stream, external_stream );
>>
>>which crashes because "stream" has not been initialized and contains
>>some junk.
>>
>>That fontforge calls FT_Open_Face() with an invalid library handle
>>might be a fontforge bug but freetype2 should not try to free "stream"
>>if "stream" has not been allocated.
>>
>>Initializing stream with NULL in FT_Open_Face() fixes this.
>
>Thank you, I will check your patch and fix the bug
>within 48 hours, please wait.
>
>Regards,
>mpsuzuki