GNOME Bugzilla – Bug 547950
gstrfuncs.c: integer overflows and buffer overruns
Last modified: 2018-05-24 11:30:43 UTC
The code in glib/gstrfuncs.c is not very robust. It causes undefined behavior for extreme but perfectly valid parameters. 1. Trying to store the result of strlen() in a variable of type "int" can cause an integer overflow on any 64-bit platform. See g_str_has_prefix() and g_str_has_suffix(). 2. Especially on a 32-bit platform adding or multiplying size_t values can easily cause a wrap around and result in allocating too little memory, resulting in a buffer overrun afterwards. Instead size_t values should instead saturate at SIZE_MAX. Assuming SIZE_MAX memory cannot be allocated, the application will properly terminate.
Created attachment 116695 [details] [review] Partial bugfix The patch fixes some of the mentioned issues but shouldn't be considered a complete fix.
Some review comments: - Coding Style: missing spaces before ( - Use gsize, G_MAXSIZE etc - add unit tests in glib/tests/strfunc.c
Review of attachment 116695 [details] [review]: This patch isn’t really going anywhere. In order for it to be accepted, I’d like to see: • Unit tests covering all the possible overflow cases (because that code is not going to be hit – and therefore not going to be tested – at any other time, normally) • Handling of overflow by returning errors, rather than saturating the size and truncating strings (which is not detectable by the caller) • Low performance overheads for any checks, since they cover really exceptional cases ::: gstrfuncs.c.orig @@ +245,3 @@ + +static inline size_t +size_smult(size_t a, size_t b) We now have g_size_checked_mul() as API, which might be useful here.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/156.