The CFF-related heap corruption vulnerability reported at https://savannah.nongnu.org/bugs/?43658 was caused by lack of function exit code validation and propagation (see the fix at http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=5f201ab5c24cb69bc96b724fd66e739928d6c5e2).

This might imply that the project code base might have more instances of function calls with unchecked return values, potentially denoting error conditions. Continuing execution in the caller function could then lead to operating on inconsistent memory state (based on the false assumption that all callees have succeeded) and consequently to various memory errors, often security related.

Fortunately, FreeType has its own type for completion status: "FT_Error" defined in include/fttypes.h:

---
287:  /*************************************************************************/
288:  /*                                                                       */
289:  /* <Type>                                                                */
290:  /*    FT_Error                                                           */
291:  /*                                                                       */
292:  /* <Description>                                                         */
293:  /*    The FreeType error code type.  A value of~0 is always interpreted  */
294:  /*    as a successful operation.                                         */
295:  /*                                                                       */
296:  typedef int  FT_Error;
---

Thanks to this, it is relatively easy to use the gcc/clang compiler to point out all occurrences of unchecked FT_Error return values by replacing line 296 with:

296:  #define FT_Error int __attribute__((warn_unused_result))

and adding the -Wno-attributes switch (in case of gcc):

$ CFLAGS=-Wno-attributes ./configure

When we then run "make", gcc will print out warnings of the following format:

---
freetype2/src/base/ftbitmap.c: In function 'FT_Bitmap_Embolden':
freetype2/src/base/ftbitmap.c:303:23: warning: ignoring return value of 'FT_Bitmap_Done', declared with attribute warn_unused_result [-Wunused-result]
         FT_Bitmap_Done( library, bitmap );
                       ^
freetype2/src/base/ftglyph.c: In function 'ft_bitmap_glyph_done':
freetype2/src/base/ftglyph.c:117:19: warning: ignoring return value of 'FT_Bitmap_Done', declared with attribute warn_unused_result [-Wunused-result]
     FT_Bitmap_Done( library, &glyph->bitmap );
---

Logs indicate that across the entire codebase (from git master as of today) reachable by the compiler on a GNU/Linux system, there are a total of 100 instances of unchecked FT_Error:

$ make 2>&1 | grep "ignoring return value" | wc -l
100

A complete list of the warnings is attached. As "FT_Error" is designed specifically to pass error codes around, we assume it should be generally checked and propagated for all functions which return it. Of course not all unchecked function calls result in security issues or even bugs, but properly handling exit codes is a good practice and a way to prevent bugs in the future (and some of them in fact can lead to security vulnerabilities, as Savannah bug #43658 showed).

I am filing a bug for this to check with the FreeType maintainers if they are interested in cleaning up this class of bugs across the code base. The specific warnings have not been investigated to determine whether any of them represent an actual security issue, but we assume there very likely are some among the 100 warnings.

After FT_Error, we could also apply the "warn_unused_result" attribute to other FreeType types; however, they are less important and less likely to exhibit actual bugs, because there might be legitimate reasons to ignore such return values.
 

unchecked_ft_error.txt 14.9 KB View Download