commit ae4f7294f83ae13e01ce66552bed041abed89377
parent 58fe386c9263f65c04917d41a074e540e75f59c5
Author: Christian Grothoff <christian@grothoff.org>
Date: Sun, 1 Nov 2026 16:32:35 +0100
satisfy contract: data_len = strlen(data)+1
Diffstat:
11 files changed, 110 insertions(+), 31 deletions(-)
diff --git a/src/main/extractor_datasource.c b/src/main/extractor_datasource.c
@@ -514,7 +514,7 @@ cfs_init_decompressor_zlib (struct CompressedFileSource *cfs,
(0 != proc (proc_cls, "<zlib>", EXTRACTOR_METATYPE_FILENAME,
EXTRACTOR_METAFORMAT_C_STRING, "text/plain",
fname,
- len)) )
+ len + 1)) ) /* C_STRING: data_len includes the 0 */
return 0; /* done */
gzip_header_length += len + 1;
}
@@ -549,7 +549,7 @@ cfs_init_decompressor_zlib (struct CompressedFileSource *cfs,
(0 != proc (proc_cls, "<zlib>", EXTRACTOR_METATYPE_COMMENT,
EXTRACTOR_METAFORMAT_C_STRING, "text/plain",
(const char *) fcomment,
- len)) )
+ len + 1)) ) /* C_STRING: data_len includes the 0 */
return 0; /* done */
gzip_header_length += len + 1;
}
diff --git a/src/plugins/applefile_extractor.c b/src/plugins/applefile_extractor.c
@@ -216,7 +216,7 @@ EXTRACTOR_applefile_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
s,
- (size_t) got + 1))
+ strlen (s) + 1))
{
free (s);
return;
diff --git a/src/plugins/dvi_extractor.c b/src/plugins/dvi_extractor.c
@@ -108,7 +108,7 @@ parseZZZ (const char *data,
EXTRACTOR_METAFORMAT_C_STRING,
"text/plain",
value,
- slen + 1))
+ strlen (value) + 1))
return 1;
}
pos += slen + 1;
@@ -273,7 +273,7 @@ EXTRACTOR_dvi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METAFORMAT_C_STRING,
"text/plain",
comment,
- klen + 1))
+ strlen (comment) + 1))
goto CLEANUP;
}
/* try to find PDF/ps special */
diff --git a/src/plugins/gif_extractor.c b/src/plugins/gif_extractor.c
@@ -125,13 +125,26 @@ EXTRACTOR_gif_extract_method (struct EXTRACTOR_ExtractContext *ec)
continue;
if (COMMENT_EXT_FUNC_CODE == et)
{
- ec->proc (ec->cls,
- "gif",
- EXTRACTOR_METATYPE_COMMENT,
- EXTRACTOR_METAFORMAT_C_STRING,
- "text/plain",
- (char*) &ext[1],
- (uint8_t) ext[0]);
+ /* The comment extension block is a *counted* string: ext[0] is the
+ number of bytes that follow and the payload is not 0-terminated.
+ Copy it out and terminate it, as C_STRING requires "data_len ==
+ strlen (data) + 1"; using strlen() on the copy also keeps the
+ invariant if the file embeds a 0 byte in the comment. */
+ char comment[256];
+ size_t clen = (uint8_t) ext[0];
+
+ memcpy (comment,
+ &ext[1],
+ clen);
+ comment[clen] = '\0';
+ if (0 != strlen (comment))
+ ec->proc (ec->cls,
+ "gif",
+ EXTRACTOR_METATYPE_COMMENT,
+ EXTRACTOR_METAFORMAT_C_STRING,
+ "text/plain",
+ comment,
+ strlen (comment) + 1);
break;
}
while ( (GIF_ERROR !=
diff --git a/src/plugins/jpeg_extractor.c b/src/plugins/jpeg_extractor.c
@@ -157,6 +157,8 @@ EXTRACTOR_jpeg_extract_method (struct EXTRACTOR_ExtractContext *ec)
for (mptr = jds.marker_list; NULL != mptr; mptr = mptr->next)
{
size_t off;
+ size_t clen;
+ char *comment;
if (JPEG_COM != mptr->marker)
continue;
@@ -165,15 +167,37 @@ EXTRACTOR_jpeg_extract_method (struct EXTRACTOR_ExtractContext *ec)
(isspace (((const unsigned char *) mptr->data)[mptr->data_length
- 1 - off])) )
off++;
+ clen = mptr->data_length - off;
+ if (0 == clen)
+ continue;
+ /* libjpeg hands out the raw marker payload, which is neither
+ 0-terminated nor guaranteed to be free of 0 bytes; C_STRING
+ promises "data_len == strlen (data) + 1", so terminate a copy
+ and measure that. */
+ if (NULL == (comment = malloc (clen + 1)))
+ goto EXIT;
+ memcpy (comment,
+ mptr->data,
+ clen);
+ comment[clen] = '\0';
+ if (0 == strlen (comment))
+ {
+ free (comment);
+ continue;
+ }
if (0 !=
ec->proc (ec->cls,
"jpeg",
EXTRACTOR_METATYPE_COMMENT,
EXTRACTOR_METAFORMAT_C_STRING,
"text/plain",
- (const char *) mptr->data,
- mptr->data_length - off))
+ comment,
+ strlen (comment) + 1))
+ {
+ free (comment);
goto EXIT;
+ }
+ free (comment);
}
EXIT:
diff --git a/src/plugins/midi_extractor.c b/src/plugins/midi_extractor.c
@@ -69,6 +69,7 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
smf_t *m = NULL;
smf_event_t *event;
uint8_t len;
+ char value[256];
if (4 >= (iret = ec->read (ec->cls, &buf, 1024)))
return;
@@ -127,6 +128,16 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
continue; /* heuristic to not get instruments */
if (0 == len)
continue;
+ /* MIDI meta events carry a *counted*, not 0-terminated string;
+ copy it out and terminate it so that we can honour the
+ "data_len == strlen (data) + 1" contract of METAFORMAT_UTF8.
+ 'len' is a uint8_t, so 256 bytes always suffice. */
+ memcpy (value,
+ &event->midi_buffer[3],
+ len);
+ value[len] = '\0';
+ if (0 == strlen (value))
+ continue; /* embedded 0 byte at the very start: nothing to report */
switch (event->midi_buffer[1])
{
case ET_TEXT_EVENT:
@@ -135,8 +146,8 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METATYPE_COMMENT,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
- (void*) &event->midi_buffer[3],
- len))
+ value,
+ strlen (value) + 1))
goto CLEANUP;
break;
case ET_COPYRIGHT_NOTICE:
@@ -145,8 +156,8 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METATYPE_COPYRIGHT,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
- (void*) &event->midi_buffer[3],
- len))
+ value,
+ strlen (value) + 1))
goto CLEANUP;
break;
case ET_TRACK_NAME:
@@ -155,8 +166,8 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METATYPE_TITLE,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
- (void*) &event->midi_buffer[3],
- len))
+ value,
+ strlen (value) + 1))
goto CLEANUP;
break;
case ET_INSTRUMENT_NAME:
@@ -165,8 +176,8 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METATYPE_SOURCE_DEVICE,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
- (void*) &event->midi_buffer[3],
- len))
+ value,
+ strlen (value) + 1))
goto CLEANUP;
break;
case ET_LYRIC_TEXT:
@@ -175,8 +186,8 @@ EXTRACTOR_midi_extract_method (struct EXTRACTOR_ExtractContext *ec)
EXTRACTOR_METATYPE_LYRICS,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
- (void*) &event->midi_buffer[3],
- len))
+ value,
+ strlen (value) + 1))
goto CLEANUP;
break;
default:
diff --git a/src/plugins/riff_extractor.c b/src/plugins/riff_extractor.c
@@ -243,7 +243,7 @@ parse_list_info (struct EXTRACTOR_ExtractContext *ec,
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
buf,
- slen + 1))
+ strlen (buf) + 1))
return 1;
break;
}
diff --git a/src/plugins/test_gif.c b/src/plugins/test_gif.c
@@ -58,7 +58,7 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_C_STRING,
"text/plain",
"Testing keyword extraction\n",
- strlen ("Testing keyword extraction\n"),
+ strlen ("Testing keyword extraction\n") + 1,
0
},
{ 0, 0, NULL, NULL, 0, -1 }
diff --git a/src/plugins/test_jpeg.c b/src/plugins/test_jpeg.c
@@ -58,7 +58,7 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_C_STRING,
"text/plain",
"(C) 2001 by Christian Grothoff, using gimp 1.2 1",
- strlen ("(C) 2001 by Christian Grothoff, using gimp 1.2 1"),
+ strlen ("(C) 2001 by Christian Grothoff, using gimp 1.2 1") + 1,
0
},
{ 0, 0, NULL, NULL, 0, -1 }
diff --git a/src/plugins/test_lib.c b/src/plugins/test_lib.c
@@ -55,6 +55,36 @@ process_replies (void *cls,
struct SolutionData *sd = cls;
unsigned int i;
+ /* Enforce the API contract documented for EXTRACTOR_METAFORMAT_UTF8
+ and EXTRACTOR_METAFORMAT_C_STRING in extractor.h: the value is a
+ 0-terminated string and "data_len" is strlen (data) + 1. A plugin
+ that reports a counted, non-terminated buffer makes every consumer
+ read out of bounds, so fail loudly instead of quietly mismatching. */
+ if ( (EXTRACTOR_METAFORMAT_UTF8 == format) ||
+ (EXTRACTOR_METAFORMAT_C_STRING == format) )
+ {
+ const char *zero = (0 == data_len)
+ ? NULL
+ : memchr (data, '\0', data_len);
+
+ /* Note: deliberately bounded by data_len; calling strlen() here
+ would itself read out of bounds for the violation we detect. */
+ if ( (NULL == zero) ||
+ (zero != &data[data_len - 1]) )
+ {
+ fprintf (stderr,
+ "Plugin `%s' violated the data_len invariant for meta type %d:"
+ " data_len is %llu, but the 0-terminator is %s\n",
+ plugin_name,
+ (int) type,
+ (unsigned long long) data_len,
+ (NULL == zero)
+ ? "absent"
+ : "not the last byte");
+ abort ();
+ }
+ }
+
for (i = 0; -1 != sd[i].solved; i++)
{
if ( (0 != sd[i].solved) ||
diff --git a/src/plugins/test_midi.c b/src/plugins/test_midi.c
@@ -50,7 +50,7 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
"(c) 2012 d-o-o",
- strlen ("(c) 2012 d-o-o"),
+ strlen ("(c) 2012 d-o-o") + 1,
0
},
{
@@ -58,7 +58,7 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
"Tage wie diese T2",
- strlen ("Tage wie diese T2"),
+ strlen ("Tage wie diese T2") + 1,
0
},
{
@@ -66,7 +66,7 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
"XFhd:::Rock:8 Beat:1:m1:-:-:-:-:DD",
- strlen ("XFhd:::Rock:8 Beat:1:m1:-:-:-:-:DD"),
+ strlen ("XFhd:::Rock:8 Beat:1:m1:-:-:-:-:DD") + 1,
0
},
{
@@ -74,7 +74,8 @@ main (int argc, char *argv[])
EXTRACTOR_METAFORMAT_UTF8,
"text/plain",
"XFln:L1:Tage wie diese:von Holst:von Holst:-:Toten Hosen:DD",
- strlen ("XFln:L1:Tage wie diese:von Holst:von Holst:-:Toten Hosen:DD"),
+ strlen (
+ "XFln:L1:Tage wie diese:von Holst:von Holst:-:Toten Hosen:DD") + 1,
0
},
{ 0, 0, NULL, NULL, 0, -1 }
