gstring: Gracefully handle NULL parameters in inline append

We could have unguarded crashes when calling strlen (NULL) or when passing
invalid GString's.

Also ensure that we are not using the macro `val` argument multiple times as
it may lead to an unwanted behavior when passing to it a variable value such
as `str[++i]`, as the value may be called multiple times.
C++ tests and Coverity were both underlining this.

Fixes: #2890
This commit is contained in:
Marco Trevisan (Treviño) 2023-01-18 18:14:10 +01:00
parent f21772ead0
commit 4ca90af6ed
3 changed files with 173 additions and 2 deletions

View File

@ -34,6 +34,7 @@
#include <glib/gtypes.h> #include <glib/gtypes.h>
#include <glib/gunicode.h> #include <glib/gunicode.h>
#include <glib/gbytes.h> #include <glib/gbytes.h>
#include <glib/gstrfuncs.h>
#include <glib/gutils.h> /* for G_CAN_INLINE */ #include <glib/gutils.h> /* for G_CAN_INLINE */
#include <string.h> #include <string.h>
@ -188,6 +189,12 @@ g_string_append_len_inline (GString *gstring,
const char *val, const char *val,
gssize len) gssize len)
{ {
if G_UNLIKELY (gstring == NULL)
return g_string_append_len (gstring, val, len);
if G_UNLIKELY (val == NULL)
return (len != 0) ? g_string_append_len (gstring, val, len) : gstring;
if (len < 0) if (len < 0)
len = strlen (val); len = strlen (val);
@ -221,9 +228,19 @@ g_string_truncate_inline (GString *gstring,
#if G_GNUC_CHECK_VERSION (2, 0) #if G_GNUC_CHECK_VERSION (2, 0)
#define g_string_append(gstr,val) g_string_append_len (gstr, val, __builtin_constant_p (val) ? (gssize) strlen (val) : (gssize) -1) #define g_string_append(gstr, val) \
(__builtin_constant_p (val) ? \
G_GNUC_EXTENSION ({ \
const char * const __val = (val); \
g_string_append_len (gstr, __val, \
G_LIKELY (__val != NULL) ? \
(gssize) strlen (_G_STR_NONNULL (__val)) \
: (gssize) -1); \
}) \
: \
g_string_append_len (gstr, val, (gssize) -1))
#endif #endif /* G_GNUC_CHECK_VERSION (2, 0) */
#endif /* G_CAN_INLINE */ #endif /* G_CAN_INLINE */

View File

@ -321,6 +321,106 @@ test_str_equal (void)
g_free (str_b); g_free (str_b);
} }
static void
test_string_append (void)
{
GString *string;
char *tmp;
int i;
tmp = g_strdup ("more");
/* append */
string = g_string_new ("firsthalf");
g_string_append (string, "last");
(g_string_append) (string, "half");
g_assert_cmpstr (string->str, ==, "firsthalflasthalf");
i = 0;
g_string_append (string, &tmp[i++]);
(g_string_append) (string, &tmp[i++]);
g_assert_true (i == 2);
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore");
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null (g_string_append (NULL, NULL));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null ((g_string_append) (NULL, NULL));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append (string, NULL) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append) (string, NULL) == string);
g_test_assert_expected_messages ();
g_string_free (string, TRUE);
/* append_len */
string = g_string_new ("firsthalf");
g_string_append_len (string, "lasthalfjunkjunk", strlen ("last"));
(g_string_append_len) (string, "halfjunkjunk", strlen ("half"));
g_string_append_len (string, "more", -1);
(g_string_append_len) (string, "ore", -1);
g_assert_true (g_string_append_len (string, NULL, 0) == string);
g_assert_true ((g_string_append_len) (string, NULL, 0) == string);
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null (g_string_append_len (NULL, NULL, -1));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null ((g_string_append_len) (NULL, NULL, -1));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append_len (string, NULL, -1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append_len) (string, NULL, -1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append_len (string, NULL, 1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append_len) (string, NULL, 1) == string);
g_test_assert_expected_messages ();
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore");
char c = 'A';
g_string_append_c (string, c++);
(g_string_append_c) (string, c++);
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreoreAB");
i = string->len;
g_string_truncate (string, --i);
(g_string_truncate) (string, --i);
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore");
g_string_free (string, TRUE);
}
int int
main (int argc, char *argv[]) main (int argc, char *argv[])
{ {
@ -344,6 +444,7 @@ main (int argc, char *argv[])
g_test_add_func ("/C++/clear-pointer", test_clear_pointer); g_test_add_func ("/C++/clear-pointer", test_clear_pointer);
g_test_add_func ("/C++/steal-pointer", test_steal_pointer); g_test_add_func ("/C++/steal-pointer", test_steal_pointer);
g_test_add_func ("/C++/str-equal", test_str_equal); g_test_add_func ("/C++/str-equal", test_str_equal);
g_test_add_func ("/C++/string-append", test_string_append);
return g_test_run (); return g_test_run ();
} }

View File

@ -225,6 +225,26 @@ test_string_append (void)
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore"); g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore");
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null (g_string_append (NULL, NULL));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null ((g_string_append) (NULL, NULL));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append (string, NULL) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append) (string, NULL) == string);
g_test_assert_expected_messages ();
g_string_free (string, TRUE); g_string_free (string, TRUE);
/* append_len */ /* append_len */
@ -234,6 +254,39 @@ test_string_append (void)
g_string_append_len (string, "more", -1); g_string_append_len (string, "more", -1);
(g_string_append_len) (string, "ore", -1); (g_string_append_len) (string, "ore", -1);
g_assert_true (g_string_append_len (string, NULL, 0) == string);
g_assert_true ((g_string_append_len) (string, NULL, 0) == string);
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null (g_string_append_len (NULL, NULL, -1));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*string != NULL*failed*");
g_assert_null ((g_string_append_len) (NULL, NULL, -1));
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append_len (string, NULL, -1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append_len) (string, NULL, -1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true (g_string_append_len (string, NULL, 1) == string);
g_test_assert_expected_messages ();
g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
"*assertion*val != NULL*failed*");
g_assert_true ((g_string_append_len) (string, NULL, 1) == string);
g_test_assert_expected_messages ();
g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore"); g_assert_cmpstr (string->str, ==, "firsthalflasthalfmoreore");
g_string_free (string, TRUE); g_string_free (string, TRUE);
} }