Add new API for splitting an untrusted string

Using fu_common_strnsplit() has the drawback that a malicious user (or
a fuzzer!) could create a file with 5,000,000 newlines, and then pass
that into any parser that tokenizes into lines. This causes millions of
tiny allocations and quickly dirties hundreds of megabytes of RSS due
to heap overheads.

Rather than splitting a huge array and then processing each line, set
up a callback to process each line and only allocate the next string if
the token was parsed correctly. This means that we don't even dup the
buffer before we start parsing, rather than allocating everything and
then failing at the first hurdle.

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=38696

libfwupdplugin/fu-common.c

@@ -1941,6 +1941,79 @@ fu_common_strnsplit(const gchar *str, gsize sz, const gchar *delimiter, gint max
 	return g_strsplit(str, delimiter, max_tokens);
 }
 
+/**
+ * fu_common_strnsplit_full:
+ * @str: a string to split
+ * @sz: size of @str, or -1 for unknown
+ * @delimiter: a string which specifies the places at which to split the string
+ * @callback: (scope call): a #FuCommonStrsplitFunc.
+ * @user_data: user data
+ * @error: (nullable): optional return location for an error
+ *
+ * Splits the string, calling the given function for each
+ * of the tokens found. If any @callback returns %FALSE scanning is aborted.
+ *
+ * Use this function in preference to fu_common_strnsplit() when the input file is untrusted,
+ * and you don't want to allocate a GStrv with billions of one byte items.
+ *
+ * Returns: %TRUE if no @callback returned FALSE
+ *
+ * Since: 1.7.0
+ */
+gboolean
+fu_common_strnsplit_full(const gchar *str,
+			 gssize sz,
+			 const gchar *delimiter,
+			 FuCommonStrsplitFunc callback,
+			 gpointer user_data,
+			 GError **error)
+{
+	gsize delimiter_sz;
+	gsize str_sz;
+	guint found_idx = 0;
+	guint token_idx = 0;
+
+	g_return_val_if_fail(str != NULL, FALSE);
+	g_return_val_if_fail(delimiter != NULL && delimiter[0] != '\0', FALSE);
+	g_return_val_if_fail(callback != NULL, FALSE);
+	g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
+
+	/* make known */
+	str_sz = sz != -1 ? (gsize)sz : strlen(str);
+	delimiter_sz = strlen(delimiter);
+
+	/* cannot split */
+	if (delimiter_sz > str_sz) {
+		g_autoptr(GString) token = g_string_new(str);
+		return callback(token, token_idx, user_data, error);
+	}
+
+	/* start splittin' */
+	for (gsize i = 0; i < (str_sz - delimiter_sz) + 1;) {
+		if (strncmp(str + i, delimiter, delimiter_sz) == 0) {
+			g_autoptr(GString) token = g_string_new(NULL);
+			g_string_append_len(token, str + found_idx, i - found_idx);
+			if (!callback(token, token_idx++, user_data, error))
+				return FALSE;
+			i += delimiter_sz;
+			found_idx = i;
+		} else {
+			i++;
+		}
+	}
+
+	/* any bits left over? */
+	if (found_idx != str_sz) {
+		g_autoptr(GString) token = g_string_new(NULL);
+		g_string_append_len(token, str + found_idx, str_sz - found_idx);
+		if (!callback(token, token_idx, user_data, error))
+			return FALSE;
+	}
+
+	/* success */
+	return TRUE;
+}
+
 /**
  * fu_common_strsafe:
  * @str: (nullable): a string to make safe for printing

libfwupdplugin/fu-common.h

@@ -344,6 +344,28 @@ void
 fu_common_string_append_kb(GString *str, guint idt, const gchar *key, gboolean value);
 gchar **
 fu_common_strnsplit(const gchar *str, gsize sz, const gchar *delimiter, gint max_tokens);
+
+/**
+ * FuCommonStrsplitFunc:
+ * @token: a #GString
+ * @token_idx: the token number
+ * @user_data: user data
+ * @error: a #GError or NULL
+ *
+ * The fu_common_strnsplit_full() iteration callback.
+ */
+typedef gboolean (*FuCommonStrsplitFunc)(GString *token,
+					 guint token_idx,
+					 gpointer user_data,
+					 GError **error);
+gboolean
+fu_common_strnsplit_full(const gchar *str,
+			 gssize sz,
+			 const gchar *delimiter,
+			 FuCommonStrsplitFunc callback,
+			 gpointer user_data,
+			 GError **error);
+
 gchar *
 fu_common_strsafe(const gchar *str, gsize maxsz);
 gchar *

libfwupdplugin/fu-ihex-firmware.c

@@ -30,6 +30,8 @@ typedef struct {
 G_DEFINE_TYPE_WITH_PRIVATE(FuIhexFirmware, fu_ihex_firmware, FU_TYPE_FIRMWARE)
 #define GET_PRIVATE(o) (fu_ihex_firmware_get_instance_private(o))
 
+#define FU_IHEX_FIRMWARE_TOKENS_MAX 100000 /* lines */
+
 /**
  * fu_ihex_firmware_get_records:
  * @self: A #FuIhexFirmware
@@ -180,30 +182,60 @@ fu_ihex_firmware_record_type_to_string(guint8 record_type)
 	return NULL;
 }
 
+typedef struct {
+	FuIhexFirmware *self;
+	FwupdInstallFlags flags;
+} FuIhexFirmwareTokenHelper;
+
+static gboolean
+fu_ihex_firmware_tokenize_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
+{
+	FuIhexFirmwareTokenHelper *helper = (FuIhexFirmwareTokenHelper *)user_data;
+	FuIhexFirmwarePrivate *priv = GET_PRIVATE(helper->self);
+	g_autoptr(FuIhexFirmwareRecord) rcd = NULL;
+
+	/* sanity check */
+	if (token_idx > FU_IHEX_FIRMWARE_TOKENS_MAX) {
+		g_set_error_literal(error,
+				    G_IO_ERROR,
+				    G_IO_ERROR_INVALID_DATA,
+				    "file has too many lines");
+		return FALSE;
+	}
+
+	/* remove WIN32 line endings */
+	g_strdelimit(token->str, "\r\x1a", '\0');
+	token->len = strlen(token->str);
+
+	/* ignore blank lines */
+	if (token->len == 0)
+		return TRUE;
+
+	/* ignore comments */
+	if (g_str_has_prefix(token->str, ";"))
+		return TRUE;
+
+	/* parse record */
+	rcd = fu_ihex_firmware_record_new(token_idx + 1, token->str, helper->flags, error);
+	if (rcd == NULL) {
+		g_prefix_error(error, "invalid line %u: ", token_idx + 1);
+		return FALSE;
+	}
+	g_ptr_array_add(priv->records, g_steal_pointer(&rcd));
+	return TRUE;
+}
+
 static gboolean
 fu_ihex_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags flags, GError **error)
 {
 	FuIhexFirmware *self = FU_IHEX_FIRMWARE(firmware);
-	FuIhexFirmwarePrivate *priv = GET_PRIVATE(self);
+	FuIhexFirmwareTokenHelper helper = {.self = self, .flags = flags};
-	gsize sz = 0;
+	return fu_common_strnsplit_full(g_bytes_get_data(fw, NULL),
-	const gchar *data = g_bytes_get_data(fw, &sz);
+					g_bytes_get_size(fw),
-	g_auto(GStrv) lines = fu_common_strnsplit(data, sz, "\n", -1);
+					"\n",
-
+					fu_ihex_firmware_tokenize_cb,
-	for (guint ln = 0; lines[ln] != NULL; ln++) {
+					&helper,
-		g_autoptr(FuIhexFirmwareRecord) rcd = NULL;
+					error);
-		g_strdelimit(lines[ln], "\r\x1a", '\0');
-		if (g_str_has_prefix(lines[ln], ";"))
-			continue;
-		if (lines[ln][0] == '\0')
-			continue;
-		rcd = fu_ihex_firmware_record_new(ln + 1, lines[ln], flags, error);
-		if (rcd == NULL) {
-			g_prefix_error(error, "invalid line %u: ", ln + 1);
-			return FALSE;
-		}
-		g_ptr_array_add(priv->records, g_steal_pointer(&rcd));
-	}
-	return TRUE;
 }
 
 static gboolean

libfwupdplugin/fu-self-test.c

@@ -422,6 +422,52 @@ fu_smbios_class_func(void)
 	g_assert_cmpuint(byte, ==, 16);
 }
 
+static gboolean
+_strnsplit_add_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
+{
+	GPtrArray *array = (GPtrArray *)user_data;
+	g_debug("TOKEN: [%s] (%u)", token->str, token_idx);
+	g_ptr_array_add(array, g_strdup(token->str));
+	return TRUE;
+}
+
+static gboolean
+_strnsplit_nop_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
+{
+	guint *cnt = (guint *)user_data;
+	(*cnt)++;
+	return TRUE;
+}
+
+static void
+fu_common_strnsplit_func(void)
+{
+	const gchar *str = "123foo123bar123";
+	const guint bigsz = 1024 * 1024;
+	gboolean ret;
+	guint cnt = 0;
+	g_autoptr(GError) error = NULL;
+	g_autoptr(GPtrArray) array = g_ptr_array_new_with_free_func(g_free);
+	g_autoptr(GString) bigstr = g_string_sized_new(bigsz * 2);
+
+	/* works for me */
+	ret = fu_common_strnsplit_full(str, -1, "123", _strnsplit_add_cb, array, &error);
+	g_assert_no_error(error);
+	g_assert_true(ret);
+	g_assert_cmpint(array->len, ==, 3);
+	g_assert_cmpstr(g_ptr_array_index(array, 0), ==, "");
+	g_assert_cmpstr(g_ptr_array_index(array, 1), ==, "foo");
+	g_assert_cmpstr(g_ptr_array_index(array, 2), ==, "bar");
+
+	/* lets try something insane */
+	for (guint i = 0; i < bigsz; i++)
+		g_string_append(bigstr, "X\n");
+	ret = fu_common_strnsplit_full(bigstr->str, -1, "\n", _strnsplit_nop_cb, &cnt, &error);
+	g_assert_no_error(error);
+	g_assert_true(ret);
+	g_assert_cmpint(cnt, ==, bigsz);
+}
+
 static void
 fu_common_strsafe_func(void)
 {
@@ -3642,6 +3688,7 @@ main(int argc, char **argv)
 	g_setenv("FWUPD_OFFLINE_TRIGGER", "/tmp/fwupd-self-test/system-update", TRUE);
 	g_setenv("FWUPD_LOCALSTATEDIR", "/tmp/fwupd-self-test/var", TRUE);
 
+	g_test_add_func("/fwupd/common{strnsplit}", fu_common_strnsplit_func);
 	g_test_add_func("/fwupd/progress", fu_progress_func);
 	g_test_add_func("/fwupd/progress{child}", fu_progress_child_func);
 	g_test_add_func("/fwupd/progress{parent-1-step}", fu_progress_parent_one_step_proxy_func);

libfwupdplugin/fu-srec-firmware.c

@@ -29,6 +29,8 @@ typedef struct {
 G_DEFINE_TYPE_WITH_PRIVATE(FuSrecFirmware, fu_srec_firmware, FU_TYPE_FIRMWARE)
 #define GET_PRIVATE(o) (fu_srec_firmware_get_instance_private(o))
 
+#define FU_SREC_FIRMWARE_TOKENS_MAX 100000 /* lines */
+
 /**
  * fu_srec_firmware_get_records:
  * @self: A #FuSrecFirmware
@@ -117,79 +119,85 @@ fu_srec_firmware_record_get_type(void)
 	return type_id;
 }
 
-static gboolean
+typedef struct {
-fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags flags, GError **error)
+	FuSrecFirmware *self;
-{
+	FwupdInstallFlags flags;
-	FuSrecFirmware *self = FU_SREC_FIRMWARE(firmware);
+	gboolean got_eof;
-	FuSrecFirmwarePrivate *priv = GET_PRIVATE(self);
+} FuSrecFirmwareTokenHelper;
-	const gchar *data;
-	gboolean got_eof = FALSE;
-	gsize sz = 0;
-	g_auto(GStrv) lines = NULL;
 
-	/* parse records */
+static gboolean
-	data = g_bytes_get_data(fw, &sz);
+fu_srec_firmware_tokenize_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
-	lines = fu_common_strnsplit(data, sz, "\n", -1);
+{
-	for (guint ln = 0; lines[ln] != NULL; ln++) {
+	FuSrecFirmwareTokenHelper *helper = (FuSrecFirmwareTokenHelper *)user_data;
+	FuSrecFirmwarePrivate *priv = GET_PRIVATE(helper->self);
 	g_autoptr(FuSrecFirmwareRecord) rcd = NULL;
-		const gchar *line = lines[ln];
-		gsize linesz;
 	guint32 rec_addr32;
 	guint16 rec_addr16;
 	guint8 addrsz = 0; /* bytes */
 	guint8 rec_count;  /* words */
 	guint8 rec_kind;
 
+	/* sanity check */
+	if (token_idx > FU_SREC_FIRMWARE_TOKENS_MAX) {
+		g_set_error_literal(error,
+				    G_IO_ERROR,
+				    G_IO_ERROR_INVALID_DATA,
+				    "file has too many lines");
+		return FALSE;
+	}
+
+	/* remove WIN32 line endings */
+	g_strdelimit(token->str, "\r\x1a", '\0');
+	token->len = strlen(token->str);
+
 	/* ignore blank lines */
-		g_strdelimit(lines[ln], "\r", '\0');
+	if (token->len == 0)
-		linesz = strlen(line);
+		return TRUE;
-		if (linesz == 0)
-			continue;
 
 	/* check starting token */
-		if (line[0] != 'S') {
+	if (token->str[0] != 'S' || token->len < 3) {
-			g_autofree gchar *strsafe = fu_common_strsafe(line, 3);
+		g_autofree gchar *strsafe = fu_common_strsafe(token->str, 3);
 		if (strsafe != NULL) {
 			g_set_error(error,
 				    FWUPD_ERROR,
 				    FWUPD_ERROR_INVALID_FILE,
 				    "invalid starting token, got '%s' at line %u",
 				    strsafe,
-					    ln + 1);
+				    token_idx + 1);
 			return FALSE;
 		}
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_INVALID_FILE,
 			    "invalid starting token at line %u",
-				    ln + 1);
+			    token_idx + 1);
 		return FALSE;
 	}
 
 	/* kind, count, address, (data), checksum, linefeed */
-		rec_kind = line[1] - '0';
+	rec_kind = token->str[1] - '0';
-		if (!fu_firmware_strparse_uint8_safe(line, linesz, 2, &rec_count, error))
+	if (!fu_firmware_strparse_uint8_safe(token->str, token->len, 2, &rec_count, error))
 		return FALSE;
-		if (rec_count * 2 != linesz - 4) {
+	if (rec_count * 2 != token->len - 4) {
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_INVALID_FILE,
 			    "count incomplete at line %u, "
 			    "length %u, expected %u",
-				    ln + 1,
+			    token_idx + 1,
-				    (guint)linesz - 4,
+			    (guint)token->len - 4,
 			    (guint)rec_count * 2);
 		return FALSE;
 	}
 
 	/* checksum check */
-		if ((flags & FWUPD_INSTALL_FLAG_IGNORE_CHECKSUM) == 0) {
+	if ((helper->flags & FWUPD_INSTALL_FLAG_IGNORE_CHECKSUM) == 0) {
 		guint8 rec_csum = 0;
 		guint8 rec_csum_expected;
 		for (guint8 i = 0; i < rec_count; i++) {
 			guint8 csum_tmp = 0;
-				if (!fu_firmware_strparse_uint8_safe(line,
+			if (!fu_firmware_strparse_uint8_safe(token->str,
-								     linesz,
+							     token->len,
 							     (i * 2) + 2,
 							     &csum_tmp,
 							     error))
@@ -197,8 +205,8 @@ fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags fl
 			rec_csum += csum_tmp;
 		}
 		rec_csum ^= 0xff;
-			if (!fu_firmware_strparse_uint8_safe(line,
+		if (!fu_firmware_strparse_uint8_safe(token->str,
-							     linesz,
+						     token->len,
 						     (rec_count * 2) + 2,
 						     &rec_csum_expected,
 						     error))
@@ -209,7 +217,7 @@ fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags fl
 				    FWUPD_ERROR_INVALID_FILE,
 				    "checksum incorrect line %u, "
 				    "expected %02x, got %02x",
-					    ln + 1,
+				    token_idx + 1,
 				    rec_csum_expected,
 				    rec_csum);
 			return FALSE;
@@ -232,46 +240,58 @@ fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags fl
 		break;
 	case FU_FIRMWARE_SREC_RECORD_KIND_S5_COUNT_16:
 		addrsz = 2;
-			got_eof = TRUE;
+		helper->got_eof = TRUE;
 		break;
 	case FU_FIRMWARE_SREC_RECORD_KIND_S6_COUNT_24:
 		addrsz = 3;
 		break;
 	case FU_FIRMWARE_SREC_RECORD_KIND_S7_COUNT_32:
 		addrsz = 4;
-			got_eof = TRUE;
+		helper->got_eof = TRUE;
 		break;
 	case FU_FIRMWARE_SREC_RECORD_KIND_S8_TERMINATION_24:
 		addrsz = 3;
-			got_eof = TRUE;
+		helper->got_eof = TRUE;
 		break;
 	case FU_FIRMWARE_SREC_RECORD_KIND_S9_TERMINATION_16:
 		addrsz = 2;
-			got_eof = TRUE;
+		helper->got_eof = TRUE;
 		break;
 	default:
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_INVALID_FILE,
 			    "invalid srec record type S%c at line %u",
-				    line[1],
+			    token->str[1],
-				    ln + 1);
+			    token_idx + 1);
 		return FALSE;
 	}
 
 	/* parse address */
 	switch (addrsz) {
 	case 2:
-			if (!fu_firmware_strparse_uint16_safe(line, linesz, 4, &rec_addr16, error))
+		if (!fu_firmware_strparse_uint16_safe(token->str,
+						      token->len,
+						      4,
+						      &rec_addr16,
+						      error))
 			return FALSE;
 		rec_addr32 = rec_addr16;
 		break;
 	case 3:
-			if (!fu_firmware_strparse_uint24_safe(line, linesz, 4, &rec_addr32, error))
+		if (!fu_firmware_strparse_uint24_safe(token->str,
+						      token->len,
+						      4,
+						      &rec_addr32,
+						      error))
 			return FALSE;
 		break;
 	case 4:
-			if (!fu_firmware_strparse_uint32_safe(line, linesz, 4, &rec_addr32, error))
+		if (!fu_firmware_strparse_uint32_safe(token->str,
+						      token->len,
+						      4,
+						      &rec_addr32,
+						      error))
 			return FALSE;
 		break;
 	default:
@@ -279,27 +299,47 @@ fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags fl
 	}
 	if (g_getenv("FU_SREC_FIRMWARE_VERBOSE") != NULL) {
 		g_debug("line %03u S%u addr:0x%04x datalen:0x%02x",
-				ln + 1,
+			token_idx + 1,
 			rec_kind,
 			rec_addr32,
 			(guint)rec_count - addrsz - 1);
 	}
 
 	/* data */
-		rcd = fu_srec_firmware_record_new(ln + 1, rec_kind, rec_addr32);
+	rcd = fu_srec_firmware_record_new(token_idx + 1, rec_kind, rec_addr32);
 	if (rec_kind == 1 || rec_kind == 2 || rec_kind == 3) {
 		for (gsize i = 4 + (addrsz * 2); i <= rec_count * 2; i += 2) {
 			guint8 tmp = 0;
-				if (!fu_firmware_strparse_uint8_safe(line, linesz, i, &tmp, error))
+			if (!fu_firmware_strparse_uint8_safe(token->str,
+							     token->len,
+							     i,
+							     &tmp,
+							     error))
 				return FALSE;
 			fu_byte_array_append_uint8(rcd->buf, tmp);
 		}
 	}
 	g_ptr_array_add(priv->records, g_steal_pointer(&rcd));
-	}
+	return TRUE;
+}
+
+static gboolean
+fu_srec_firmware_tokenize(FuFirmware *firmware, GBytes *fw, FwupdInstallFlags flags, GError **error)
+{
+	FuSrecFirmware *self = FU_SREC_FIRMWARE(firmware);
+	FuSrecFirmwareTokenHelper helper = {.self = self, .flags = flags, .got_eof = FALSE};
+
+	/* parse records */
+	if (!fu_common_strnsplit_full(g_bytes_get_data(fw, NULL),
+				      g_bytes_get_size(fw),
+				      "\n",
+				      fu_srec_firmware_tokenize_cb,
+				      &helper,
+				      error))
+		return FALSE;
 
 	/* no EOF */
-	if (!got_eof) {
+	if (!helper.got_eof) {
 		g_set_error_literal(error,
 				    FWUPD_ERROR,
 				    FWUPD_ERROR_INVALID_FILE,

libfwupdplugin/fwupdplugin.map

@@ -861,6 +861,7 @@ LIBFWUPDPLUGIN_1.6.2 {
 
 LIBFWUPDPLUGIN_1.7.0 {
   global:
+    fu_common_strnsplit_full;
     fu_device_set_progress;
     fu_plugin_runner_attach;
     fu_plugin_runner_cleanup;

plugins/ccgx/fu-ccgx-firmware.c

@@ -27,6 +27,8 @@ G_DEFINE_TYPE(FuCcgxFirmware, fu_ccgx_firmware, FU_TYPE_FIRMWARE)
 /* offset stored application version for CCGx */
 #define CCGX_APP_VERSION_OFFSET 228 /* 128+64+32+4 */
 
+#define FU_CCGX_FIRMWARE_TOKENS_MAX 100000 /* lines */
+
 GPtrArray *
 fu_ccgx_firmware_get_records(FuCcgxFirmware *self)
 {
@@ -79,11 +81,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(FuCcgxFirmwareRecord, fu_ccgx_firmware_record_free
 
 static gboolean
 fu_ccgx_firmware_add_record(FuCcgxFirmware *self,
-			    const gchar *line,
+			    GString *token,
 			    FwupdInstallFlags flags,
 			    GError **error)
 {
-	guint16 linesz = strlen(line);
 	guint16 buflen;
 	guint8 checksum_calc = 0;
 	g_autoptr(FuCcgxFirmwareRecord) rcd = NULL;
@@ -91,26 +92,30 @@ fu_ccgx_firmware_add_record(FuCcgxFirmware *self,
 
 	/* parse according to https://community.cypress.com/docs/DOC-10562 */
 	rcd = g_new0(FuCcgxFirmwareRecord, 1);
-	if (!fu_firmware_strparse_uint8_safe(line, linesz, 0, &rcd->array_id, error))
+	if (!fu_firmware_strparse_uint8_safe(token->str, token->len, 0, &rcd->array_id, error))
 		return FALSE;
-	if (!fu_firmware_strparse_uint16_safe(line, linesz, 2, &rcd->row_number, error))
+	if (!fu_firmware_strparse_uint16_safe(token->str, token->len, 2, &rcd->row_number, error))
 		return FALSE;
-	if (!fu_firmware_strparse_uint16_safe(line, linesz, 6, &buflen, error))
+	if (!fu_firmware_strparse_uint16_safe(token->str, token->len, 6, &buflen, error))
 		return FALSE;
-	if (linesz != (buflen * 2) + 12) {
+	if (token->len != ((gsize)buflen * 2) + 12) {
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_NOT_SUPPORTED,
 			    "invalid record, expected %u chars, got %u",
 			    (guint)(buflen * 2) + 12,
-			    linesz);
+			    (guint)token->len);
 		return FALSE;
 	}
 
 	/* parse payload, adding checksum */
 	for (guint i = 0; i < buflen; i++) {
 		guint8 tmp = 0;
-		if (!fu_firmware_strparse_uint8_safe(line, linesz, 10 + (i * 2), &tmp, error))
+		if (!fu_firmware_strparse_uint8_safe(token->str,
+						     token->len,
+						     10 + (i * 2),
+						     &tmp,
+						     error))
 			return FALSE;
 		fu_byte_array_append_uint8(data, tmp);
 		checksum_calc += tmp;
@@ -120,15 +125,19 @@ fu_ccgx_firmware_add_record(FuCcgxFirmware *self,
 	/* verify 2s complement checksum */
 	if ((flags & FWUPD_INSTALL_FLAG_IGNORE_CHECKSUM) == 0) {
 		guint8 checksum_file;
-		if (!fu_firmware_strparse_uint8_safe(line,
+		if (!fu_firmware_strparse_uint8_safe(token->str,
-						     linesz,
+						     token->len,
 						     (buflen * 2) + 10,
 						     &checksum_file,
 						     error))
 			return FALSE;
 		for (guint i = 0; i < 5; i++) {
 			guint8 tmp = 0;
-			if (!fu_firmware_strparse_uint8_safe(line, linesz, i * 2, &tmp, error))
+			if (!fu_firmware_strparse_uint8_safe(token->str,
+							     token->len,
+							     i * 2,
+							     &tmp,
+							     error))
 				return FALSE;
 			checksum_calc += tmp;
 		}
@@ -287,33 +296,35 @@ fu_ccgx_firmware_parse_md_block(FuCcgxFirmware *self, FwupdInstallFlags flags, G
 	return TRUE;
 }
 
-static gboolean
+typedef struct {
-fu_ccgx_firmware_parse(FuFirmware *firmware,
+	FuCcgxFirmware *self;
-		       GBytes *fw,
+	FwupdInstallFlags flags;
-		       guint64 addr_start,
+} FuCcgxFirmwareTokenHelper;
-		       guint64 addr_end,
-		       FwupdInstallFlags flags,
-		       GError **error)
-{
-	FuCcgxFirmware *self = FU_CCGX_FIRMWARE(firmware);
-	gsize linesz;
-	gsize sz = 0;
-	guint32 device_id = 0;
-	const gchar *data = g_bytes_get_data(fw, &sz);
-	g_auto(GStrv) lines = fu_common_strnsplit(data, sz, "\n", -1);
 
-	/* parse header */
+static gboolean
-	if (lines[0] == NULL) {
+fu_ccgx_firmware_tokenize_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
+{
+	FuCcgxFirmwareTokenHelper *helper = (FuCcgxFirmwareTokenHelper *)user_data;
+	FuCcgxFirmware *self = FU_CCGX_FIRMWARE(helper->self);
+
+	/* sanity check */
+	if (token_idx > FU_CCGX_FIRMWARE_TOKENS_MAX) {
 		g_set_error_literal(error,
-				    FWUPD_ERROR,
+				    G_IO_ERROR,
-				    FWUPD_ERROR_NOT_SUPPORTED,
+				    G_IO_ERROR_INVALID_DATA,
-				    "invalid header, expected == 12 chars");
+				    "file has too many lines");
 		return FALSE;
 	}
-	g_strdelimit(lines[0], "\r\x1a", '\0');
+
-	linesz = strlen(lines[0]);
+	/* remove WIN32 line endings */
-	if (linesz != 12) {
+	g_strdelimit(token->str, "\r\x1a", '\0');
-		g_autofree gchar *strsafe = fu_common_strsafe(lines[0], 12);
+	token->len = strlen(token->str);
+
+	/* header */
+	if (token_idx == 0) {
+		guint32 device_id = 0;
+		if (token->len != 12) {
+			g_autofree gchar *strsafe = fu_common_strsafe(token->str, 12);
 			if (strsafe != NULL) {
 				g_set_error(error,
 					    FWUPD_ERROR,
@@ -328,20 +339,45 @@ fu_ccgx_firmware_parse(FuFirmware *firmware,
 					    "invalid header, expected == 12 chars");
 			return FALSE;
 		}
-	if (!fu_firmware_strparse_uint32_safe(lines[0], linesz, 0, &device_id, error))
+		if (!fu_firmware_strparse_uint32_safe(token->str, token->len, 0, &device_id, error))
 			return FALSE;
 		self->silicon_id = device_id >> 16;
+		return TRUE;
+	}
 
-	/* parse data */
+	/* ignore blank lines */
-	for (guint ln = 1; lines[ln] != NULL; ln++) {
+	if (token->len == 0)
-		g_strdelimit(lines[ln], "\r\x1a", '\0');
+		return TRUE;
-		if (lines[ln][0] == '\0')
+
-			continue;
+	/* parse record */
-		if (!fu_ccgx_firmware_add_record(self, lines[ln] + 1, flags, error)) {
+	if (!fu_ccgx_firmware_add_record(self, token, helper->flags, error)) {
-			g_prefix_error(error, "error on line %u: ", ln + 1);
+		g_prefix_error(error, "error on line %u: ", token_idx + 1);
 		return FALSE;
 	}
-	}
+
+	/* success */
+	return TRUE;
+}
+
+static gboolean
+fu_ccgx_firmware_parse(FuFirmware *firmware,
+		       GBytes *fw,
+		       guint64 addr_start,
+		       guint64 addr_end,
+		       FwupdInstallFlags flags,
+		       GError **error)
+{
+	FuCcgxFirmware *self = FU_CCGX_FIRMWARE(firmware);
+	FuCcgxFirmwareTokenHelper helper = {.self = self, .flags = flags};
+
+	/* tokenize */
+	if (!fu_common_strnsplit_full(g_bytes_get_data(fw, NULL),
+				      g_bytes_get_size(fw),
+				      "\n",
+				      fu_ccgx_firmware_tokenize_cb,
+				      &helper,
+				      error))
+		return FALSE;
 
 	/* address is first data entry */
 	if (self->records->len > 0) {

plugins/wacom-usb/fu-wac-firmware.c

@@ -18,67 +18,61 @@ struct _FuWacFirmware {
 
 G_DEFINE_TYPE(FuWacFirmware, fu_wac_firmware, FU_TYPE_FIRMWARE)
 
+#define FU_WAC_FIRMWARE_TOKENS_MAX 100000 /* lines */
+
 typedef struct {
 	guint32 addr;
 	guint32 sz;
 	guint32 prog_start_addr;
 } FuFirmwareWacHeaderRecord;
 
-static gboolean
+typedef struct {
-fu_wac_firmware_parse(FuFirmware *firmware,
+	FuFirmware *firmware;
-		      GBytes *fw,
+	FwupdInstallFlags flags;
-		      guint64 addr_start,
+	GPtrArray *header_infos;
-		      guint64 addr_end,
+	GString *image_buffer;
-		      FwupdInstallFlags flags,
+	guint8 images_cnt;
-		      GError **error)
+} FuWacFirmwareTokenHelper;
-{
-	gsize len;
-	guint8 *data;
-	guint8 images_cnt = 0;
-	g_auto(GStrv) lines = NULL;
-	g_autoptr(GPtrArray) header_infos = g_ptr_array_new_with_free_func(g_free);
-	g_autoptr(GString) image_buffer = NULL;
 
-	/* check the prefix (BE) */
+static gboolean
-	data = (guint8 *)g_bytes_get_data(fw, &len);
+fu_wac_firmware_tokenize_cb(GString *token, guint token_idx, gpointer user_data, GError **error)
-	if (len < 5 || memcmp(data, "WACOM", 5) != 0) {
+{
+	FuWacFirmwareTokenHelper *helper = (FuWacFirmwareTokenHelper *)user_data;
+	g_autofree gchar *cmd = NULL;
+
+	/* sanity check */
+	if (token_idx > FU_WAC_FIRMWARE_TOKENS_MAX) {
 		g_set_error_literal(error,
-				    FWUPD_ERROR,
+				    G_IO_ERROR,
-				    FWUPD_ERROR_INTERNAL,
+				    G_IO_ERROR_INVALID_DATA,
-				    "invalid .wac prefix");
+				    "file has too many lines");
 		return FALSE;
 	}
 
-	/* parse each line */
+	/* remove WIN32 line endings */
-	lines = fu_common_strnsplit((const gchar *)data, len, "\n", -1);
+	g_strdelimit(token->str, "\r\x1a", '\0');
-	for (guint i = 0; lines[i] != NULL; i++) {
+	token->len = strlen(token->str);
-		g_autofree gchar *cmd = NULL;
-
-		/* remove windows line endings */
-		g_strdelimit(lines[i], "\r", '\0');
-		cmd = g_strndup(lines[i], 2);
 
 	/* ignore blank lines */
+	cmd = g_strndup(token->str, 2);
 	if (g_strcmp0(cmd, "") == 0)
-			continue;
+		return TRUE;
 
 	/* Wacom-specific metadata */
 	if (g_strcmp0(cmd, "WA") == 0) {
-			gsize cmdlen = strlen(lines[i]);
-
 		/* header info record */
-			if (cmdlen > 3 && memcmp(lines[i] + 2, "COM", 3) == 0) {
+		if (token->len > 3 && memcmp(token->str + 2, "COM", 3) == 0) {
 			guint8 header_image_cnt = 0;
-				if (cmdlen != 40) {
+			if (token->len != 40) {
 				g_set_error(error,
 					    FWUPD_ERROR,
 					    FWUPD_ERROR_INTERNAL,
 					    "invalid header, got %" G_GSIZE_FORMAT " bytes",
-						    cmdlen);
+					    token->len);
 				return FALSE;
 			}
-				if (!fu_firmware_strparse_uint4_safe(lines[i],
+			if (!fu_firmware_strparse_uint4_safe(token->str,
-								     cmdlen,
+							     token->len,
 							     5,
 							     &header_image_cnt,
 							     error))
@@ -86,31 +80,31 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 			for (guint j = 0; j < header_image_cnt; j++) {
 				g_autofree FuFirmwareWacHeaderRecord *hdr = NULL;
 				hdr = g_new0(FuFirmwareWacHeaderRecord, 1);
-					if (!fu_firmware_strparse_uint32_safe(lines[i],
+				if (!fu_firmware_strparse_uint32_safe(token->str,
-									      cmdlen,
+								      token->len,
 								      (j * 16) + 6,
 								      &hdr->addr,
 								      error))
 					return FALSE;
-					if (!fu_firmware_strparse_uint32_safe(lines[i],
+				if (!fu_firmware_strparse_uint32_safe(token->str,
-									      cmdlen,
+								      token->len,
 								      (j * 16) + 14,
 								      &hdr->sz,
 								      error))
 					return FALSE;
 				g_debug("header_fw%u_addr: 0x%x", j, hdr->addr);
 				g_debug("header_fw%u_sz:   0x%x", j, hdr->sz);
-					g_ptr_array_add(header_infos, g_steal_pointer(&hdr));
+				g_ptr_array_add(helper->header_infos, g_steal_pointer(&hdr));
 			}
-				continue;
+			return TRUE;
 		}
 
 		/* firmware headline record */
-			if (cmdlen == 13) {
+		if (token->len == 13) {
 			FuFirmwareWacHeaderRecord *hdr;
 			guint8 idx = 0;
-				if (!fu_firmware_strparse_uint4_safe(lines[i],
+			if (!fu_firmware_strparse_uint4_safe(token->str,
-								     cmdlen,
+							     token->len,
 							     2,
 							     &idx,
 							     error))
@@ -123,16 +117,16 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 					    idx);
 				return FALSE;
 			}
-				if (idx > header_infos->len) {
+			if (idx > helper->header_infos->len) {
 				g_set_error(error,
 					    FWUPD_ERROR,
 					    FWUPD_ERROR_INTERNAL,
 					    "headline %u exceeds header count %u",
 					    idx,
-						    header_infos->len);
+					    helper->header_infos->len);
 				return FALSE;
 			}
-				if (idx - 1 != images_cnt) {
+			if (idx - 1 != helper->images_cnt) {
 				g_set_error(error,
 					    FWUPD_ERROR,
 					    FWUPD_ERROR_INTERNAL,
@@ -140,9 +134,9 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 					    idx);
 				return FALSE;
 			}
-				hdr = g_ptr_array_index(header_infos, idx - 1);
+			hdr = g_ptr_array_index(helper->header_infos, idx - 1);
-				if (!fu_firmware_strparse_uint32_safe(lines[i],
+			if (!fu_firmware_strparse_uint32_safe(token->str,
-								      cmdlen,
+							      token->len,
 							      3,
 							      &hdr->prog_start_addr,
 							      error))
@@ -159,45 +153,41 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 				return FALSE;
 			}
 			g_debug("programing-start-address: 0x%x", hdr->prog_start_addr);
-				continue;
+			return TRUE;
 		}
 
 		g_debug("unknown Wacom-specific metadata");
-			continue;
+		return TRUE;
 	}
 
 	/* start */
 	if (g_strcmp0(cmd, "S0") == 0) {
-			if (image_buffer != NULL) {
+		if (helper->image_buffer->len > 0) {
 			g_set_error_literal(error,
 					    FWUPD_ERROR,
 					    FWUPD_ERROR_INTERNAL,
 					    "duplicate S0 without S7");
 			return FALSE;
 		}
-			image_buffer = g_string_new(NULL);
+		g_string_append_printf(helper->image_buffer, "%s\n", token->str);
+		return TRUE;
 	}
 
 	/* these are things we want to include in the image */
-		if (g_strcmp0(cmd, "S0") == 0 || g_strcmp0(cmd, "S1") == 0 ||
+	if (g_strcmp0(cmd, "S1") == 0 || g_strcmp0(cmd, "S2") == 0 || g_strcmp0(cmd, "S3") == 0 ||
-		    g_strcmp0(cmd, "S2") == 0 || g_strcmp0(cmd, "S3") == 0 ||
+	    g_strcmp0(cmd, "S5") == 0 || g_strcmp0(cmd, "S7") == 0 || g_strcmp0(cmd, "S8") == 0 ||
-		    g_strcmp0(cmd, "S5") == 0 || g_strcmp0(cmd, "S7") == 0 ||
+	    g_strcmp0(cmd, "S9") == 0) {
-		    g_strcmp0(cmd, "S8") == 0 || g_strcmp0(cmd, "S9") == 0) {
+		if (helper->image_buffer->len == 0) {
-			if (image_buffer == NULL) {
+			g_set_error(error, FWUPD_ERROR, FWUPD_ERROR_INTERNAL, "%s without S0", cmd);
-				g_set_error(error,
-					    FWUPD_ERROR,
-					    FWUPD_ERROR_INTERNAL,
-					    "%s without S0",
-					    cmd);
 			return FALSE;
 		}
-			g_string_append_printf(image_buffer, "%s\n", lines[i]);
+		g_string_append_printf(helper->image_buffer, "%s\n", token->str);
 	} else {
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_INTERNAL,
 			    "invalid SREC command on line %u: %s",
-				    i + 1,
+			    token_idx + 1,
 			    cmd);
 		return FALSE;
 	}
@@ -211,7 +201,7 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 		FuFirmwareWacHeaderRecord *hdr;
 
 		/* get the correct relocated start address */
-			if (images_cnt >= header_infos->len) {
+		if (helper->images_cnt >= helper->header_infos->len) {
 			g_set_error(error,
 				    FWUPD_ERROR,
 				    FWUPD_ERROR_INTERNAL,
@@ -219,9 +209,9 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 				    cmd);
 			return FALSE;
 		}
-			hdr = g_ptr_array_index(header_infos, images_cnt);
+		hdr = g_ptr_array_index(helper->header_infos, helper->images_cnt);
 
-			if (image_buffer == NULL) {
+		if (helper->image_buffer->len == 0) {
 			g_set_error(error,
 				    FWUPD_ERROR,
 				    FWUPD_ERROR_INTERNAL,
@@ -231,12 +221,12 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 		}
 
 		/* parse SREC file and add as image */
-			blob = g_bytes_new(image_buffer->str, image_buffer->len);
+		blob = g_bytes_new(helper->image_buffer->str, helper->image_buffer->len);
 		if (!fu_firmware_parse_full(firmware_srec,
 					    blob,
 					    hdr->addr,
 					    0x0,
-						    flags,
+					    helper->flags,
 					    error))
 			return FALSE;
 		fw_srec = fu_firmware_get_bytes(firmware_srec, error);
@@ -244,18 +234,51 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 			return FALSE;
 		fu_firmware_set_bytes(img, fw_srec);
 		fu_firmware_set_addr(img, fu_firmware_get_addr(firmware_srec));
-			fu_firmware_set_idx(img, images_cnt);
+		fu_firmware_set_idx(img, helper->images_cnt);
-			fu_firmware_add_image(firmware, img);
+		fu_firmware_add_image(helper->firmware, img);
-			images_cnt++;
+		helper->images_cnt++;
 
 		/* clear the image buffer */
-			g_string_free(image_buffer, TRUE);
+		g_string_set_size(helper->image_buffer, 0);
-			image_buffer = NULL;
-		}
 	}
 
+	/* success */
+	return TRUE;
+}
+
+static gboolean
+fu_wac_firmware_parse(FuFirmware *firmware,
+		      GBytes *fw,
+		      guint64 addr_start,
+		      guint64 addr_end,
+		      FwupdInstallFlags flags,
+		      GError **error)
+{
+	gsize sz = 0;
+	const gchar *data = g_bytes_get_data(fw, &sz);
+	g_autoptr(GPtrArray) header_infos = g_ptr_array_new_with_free_func(g_free);
+	g_autoptr(GString) image_buffer = g_string_new(NULL);
+	FuWacFirmwareTokenHelper helper = {.firmware = firmware,
+					   .flags = flags,
+					   .header_infos = header_infos,
+					   .image_buffer = image_buffer,
+					   .images_cnt = 0};
+
+	/* check the prefix (BE) */
+	if (sz < 5 || memcmp(data, "WACOM", 5) != 0) {
+		g_set_error_literal(error,
+				    FWUPD_ERROR,
+				    FWUPD_ERROR_INTERNAL,
+				    "invalid .wac prefix");
+		return FALSE;
+	}
+
+	/* tokenize */
+	if (!fu_common_strnsplit_full(data, sz, "\n", fu_wac_firmware_tokenize_cb, &helper, error))
+		return FALSE;
+
 	/* verify data is complete */
-	if (image_buffer != NULL) {
+	if (helper.image_buffer != NULL) {
 		g_set_error_literal(error,
 				    FWUPD_ERROR,
 				    FWUPD_ERROR_INTERNAL,
@@ -264,12 +287,12 @@ fu_wac_firmware_parse(FuFirmware *firmware,
 	}
 
 	/* ensure this matched the header */
-	if (header_infos->len != images_cnt) {
+	if (helper.header_infos->len != helper.images_cnt) {
 		g_set_error(error,
 			    FWUPD_ERROR,
 			    FWUPD_ERROR_INTERNAL,
 			    "not enough images %u for header count %u",
-			    images_cnt,
+			    helper.images_cnt,
 			    header_infos->len);
 		return FALSE;
 	}