On Mon, 2012-09-03 at 23:09 +0200, Max Kellermann wrote:
> On 2012/08/21 20:01, Jurgen Kramer <gtmkra...@xs4all.nl> wrote:
> > > Can you use dsdlib_tag_id3() for a DoS attack? This looks like it
> > > could easily cause a stack overflow:
> > >
> > > + count = is->size - is->offset;
> > > + id3_byte_t dsdid3[count];
> >
> > What is your concern here? The allocation of dsdid3 using count or the
> > value of count being calculated that way (or both)?
>
> The former. A malicious file can cause a stack overflow. Imagine a
> file that makes MPD try to allocate a few gigabytes of heap.
>
Attached is an old fashioned patch with the updated code against current
mpd git.
I cannot make heads or tales from the mess that is my mpd git repo.
Everything I try only seems to make it worse.
Jurgen
diff -uNrp mpd/src/decoder/dsdiff_decoder_plugin.c mpd-git-0.18-jk/src/decoder/dsdiff_decoder_plugin.c
--- mpd/src/decoder/dsdiff_decoder_plugin.c 2012-09-19 14:30:05.059747095 +0200
+++ mpd-git-0.18-jk/src/decoder/dsdiff_decoder_plugin.c 2012-09-19 14:45:04.204797373 +0200
@@ -52,10 +52,23 @@ struct dsdiff_chunk_header {
uint32_t size_high, size_low;
};
+/** struct for DSDIFF native Artist and Title tags */
+struct dsdiff_native_tag {
+ uint32_t size;
+};
+
struct dsdiff_metadata {
unsigned sample_rate, channels;
bool bitreverse;
uint64_t chunk_size;
+#ifdef HAVE_ID3TAG
+ goffset id3_offset;
+ uint64_t id3_size;
+#endif
+ /** offset for artist tag */
+ goffset diar_offset;
+ /** offset for title tag */
+ goffset diti_offset;
};
static bool lsbitfirst;
@@ -187,6 +200,122 @@ dsdiff_read_prop(struct decoder *decoder
return dsdlib_skip_to(decoder, is, end_offset);
}
+static void
+dsdiff_handle_native_tag(struct input_stream *is,
+ const struct tag_handler *handler,
+ void *handler_ctx, goffset tagoffset,
+ enum tag_type type)
+{
+ if (!dsdlib_skip_to(NULL, is, tagoffset))
+ return;
+
+ struct dsdiff_native_tag metatag;
+
+ if (!dsdlib_read(NULL, is, &metatag, sizeof(metatag)))
+ return;
+
+ uint32_t length = GUINT32_FROM_BE(metatag.size);
+ char string[length];
+ char *label;
+ label = string;
+
+ if (!dsdlib_read(NULL, is, label, (size_t)length))
+ return;
+
+ string[length] = '\0';
+ tag_handler_invoke_tag(handler, handler_ctx, type, label);
+ return;
+}
+
+/**
+ * Read and parse additional metadata chunks for tagging purposes. By default
+ * dsdiff files only support equivalents for artist and title but some of the
+ * extract tools add an id3 tag to provide more tags. If such id3 is found
+ * this will be used for tagging otherwise the native tags (if any) will be
+ * used
+ */
+
+static bool
+dsdiff_read_metadata_extra(struct decoder *decoder, struct input_stream *is,
+ struct dsdiff_metadata *metadata,
+ struct dsdiff_chunk_header *chunk_header,
+ const struct tag_handler *handler,
+ void *handler_ctx)
+{
+
+ /* skip from DSD data to next chunk header */
+ if (!dsdlib_skip(decoder, is, metadata->chunk_size))
+ return false;
+ if (!dsdiff_read_chunk_header(decoder, is, chunk_header))
+ return false;
+
+#ifdef HAVE_ID3TAG
+ metadata->id3_size = 0;
+#endif
+
+ /* Now process all the remaining chunk headers in the stream
+ and record their position and size */
+
+ while ( is->offset < is->size )
+ {
+ uint64_t chunk_size = dsdiff_chunk_size(chunk_header);
+
+ /* DIIN chunk, is directly followed by other chunks */
+ if (dsdlib_id_equals(&chunk_header->id, "DIIN"))
+ chunk_size = 0;
+
+ /* DIAR chunk - DSDIFF native tag for Artist */
+ if (dsdlib_id_equals(&chunk_header->id, "DIAR")) {
+ chunk_size = dsdiff_chunk_size(chunk_header);
+ metadata->diar_offset = is->offset;
+ }
+
+ /* DITI chunk - DSDIFF native tag for Title */
+ if (dsdlib_id_equals(&chunk_header->id, "DITI")) {
+ chunk_size = dsdiff_chunk_size(chunk_header);
+ metadata->diti_offset = is->offset;
+ }
+#ifdef HAVE_ID3TAG
+ /* 'ID3 ' chunk, offspec. Used by sacdextract */
+ if (dsdlib_id_equals(&chunk_header->id, "ID3 ")) {
+ chunk_size = dsdiff_chunk_size(chunk_header);
+ metadata->id3_offset = is->offset;
+ metadata->id3_size = chunk_size;
+ }
+#endif
+ if (chunk_size != 0) {
+ if (!dsdlib_skip(decoder, is, chunk_size))
+ break;
+ }
+
+ if ( is->offset < is->size ) {
+ if (!dsdiff_read_chunk_header(decoder, is, chunk_header))
+ return false;
+ }
+ chunk_size = 0;
+ }
+ /* done processing chunk headers, process tags if any */
+
+#ifdef HAVE_ID3TAG
+ if (metadata->id3_offset != 0)
+ {
+ /* a ID3 tag has preference over the other tags, do not process
+ other tags if we have one */
+ dsdlib_tag_id3(is, handler, handler_ctx, metadata->id3_offset);
+ return true;
+ }
+#endif
+
+ if (metadata->diar_offset != 0)
+ dsdiff_handle_native_tag(is, handler, handler_ctx,
+ metadata->diar_offset, TAG_ARTIST);
+
+ if (metadata->diti_offset != 0)
+ dsdiff_handle_native_tag(is, handler, handler_ctx,
+ metadata->diti_offset, TAG_TITLE);
+ return true;
+}
+
/**
* Read and parse all metadata chunks at the beginning. Stop when the
* first "DSD" chunk is seen, and return its header in the
@@ -374,6 +503,10 @@ dsdiff_scan_stream(struct input_stream *
metadata.sample_rate;
tag_handler_invoke_duration(handler, handler_ctx, songtime);
+ /* Read additional metadata and created tags if available */
+ dsdiff_read_metadata_extra(NULL, is, &metadata, &chunk_header,
+ handler, handler_ctx);
+
return true;
}
diff -uNrp mpd/src/decoder/dsdlib.c mpd-git-0.18-jk/src/decoder/dsdlib.c
--- mpd/src/decoder/dsdlib.c 2012-09-19 14:30:05.059747095 +0200
+++ mpd-git-0.18-jk/src/decoder/dsdlib.c 2012-09-19 14:44:54.066796806 +0200
@@ -27,12 +27,18 @@
#include "dsf_decoder_plugin.h"
#include "decoder_api.h"
#include "util/bit_reverse.h"
+#include "tag_handler.h"
+#include "tag_id3.h"
#include "dsdlib.h"
#include "dsdiff_decoder_plugin.h"
#include <unistd.h>
#include <stdio.h> /* for SEEK_SET, SEEK_CUR */
+#ifdef HAVE_ID3TAG
+#include <id3tag.h>
+#endif
+
bool
dsdlib_id_equals(const struct dsdlib_id *id, const char *s)
{
@@ -110,3 +116,52 @@ dsdlib_skip(struct decoder *decoder, str
return true;
}
+/**
+ * Add tags from ID3 tag. All tags commonly found in the ID3 tags of
+ * DSF and DSDIFF files are imported
+ */
+
+#ifdef HAVE_ID3TAG
+void
+dsdlib_tag_id3(struct input_stream *is,
+ const struct tag_handler *handler,
+ void *handler_ctx, goffset tagoffset)
+{
+ assert(taggoffset >= 0);
+
+ if (tagoffset == 0)
+ return;
+
+ if (!dsdlib_skip_to(NULL, is, tagoffset))
+ return;
+
+ struct id3_tag *id3_tag = NULL;
+ id3_length_t count;
+
+ /* Prevent broken files causing problems */
+ if (is->offset >= is->size)
+ return;
+
+ count = is->size - is->offset;
+ /* ID3 tag cannot be larger then complete file */
+ if ((unsigned)count >= is->size)
+ return;
+
+ id3_byte_t dsdid3[count];
+ id3_byte_t *dsdid3data;
+ dsdid3data = dsdid3;
+
+ if (!dsdlib_read(NULL, is, dsdid3data, count))
+ return;
+
+ id3_tag = id3_tag_parse(dsdid3data, count);
+ if (id3_tag == NULL)
+ return;
+
+ scan_id3_tag(id3_tag, handler, handler_ctx);
+
+ id3_tag_delete(id3_tag);
+
+ return;
+}
+#endif
diff -uNrp mpd/src/decoder/dsdlib.h mpd-git-0.18-jk/src/decoder/dsdlib.h
--- mpd/src/decoder/dsdlib.h 2012-09-19 14:30:05.059747095 +0200
+++ mpd-git-0.18-jk/src/decoder/dsdlib.h 2012-09-19 14:44:54.066796806 +0200
@@ -39,4 +39,9 @@ bool
dsdlib_skip(struct decoder *decoder, struct input_stream *is,
goffset delta);
+void
+dsdlib_tag_id3(struct input_stream *is,
+ const struct tag_handler *handler,
+ void *handler_ctx, goffset tagoffset);
+
#endif
diff -uNrp mpd/src/decoder/dsf_decoder_plugin.c mpd-git-0.18-jk/src/decoder/dsf_decoder_plugin.c
--- mpd/src/decoder/dsf_decoder_plugin.c 2012-09-19 14:30:05.059747095 +0200
+++ mpd-git-0.18-jk/src/decoder/dsf_decoder_plugin.c 2012-09-19 14:44:58.502797054 +0200
@@ -45,6 +45,10 @@ struct dsf_metadata {
unsigned sample_rate, channels;
bool bitreverse;
uint64_t chunk_size;
+#ifdef HAVE_ID3TAG
+ goffset id3_offset;
+ uint64_t id3_size;
+#endif
};
struct dsf_header {
@@ -57,6 +61,7 @@ struct dsf_header {
/** pointer to id3v2 metadata, should be at the end of the file */
uint32_t pmeta_low, pmeta_high;
};
+
/** DSF file fmt chunk */
struct dsf_fmt_chunk {
@@ -109,6 +114,12 @@ dsf_read_metadata(struct decoder *decode
if (sizeof(dsf_header) != chunk_size)
return false;
+#ifdef HAVE_ID3TAG
+ uint64_t metadata_offset;
+ metadata_offset = (((uint64_t)GUINT32_FROM_LE(dsf_header.pmeta_high)) << 32) |
+ ((uint64_t)GUINT32_FROM_LE(dsf_header.pmeta_low));
+#endif
+
/* read the 'fmt ' chunk of the DSF file */
struct dsf_fmt_chunk dsf_fmt_chunk;
if (!dsdlib_read(decoder, is, &dsf_fmt_chunk, sizeof(dsf_fmt_chunk)) ||
@@ -153,9 +164,19 @@ dsf_read_metadata(struct decoder *decode
data_size -= sizeof(data_chunk);
metadata->chunk_size = data_size;
+ /* data_size cannot be bigger or equal to total file size */
+ if (data_size >= (unsigned) is->size)
+ return false;
+
metadata->channels = (unsigned) dsf_fmt_chunk.channelnum;
metadata->sample_rate = samplefreq;
-
+#ifdef HAVE_ID3TAG
+ /* metada_offset cannot be bigger then or equal to total file size */
+ if (metadata_offset >= (unsigned) is->size)
+ metadata->id3_offset = 0;
+ else
+ metadata->id3_offset = (goffset) metadata_offset;
+#endif
/* check bits per sample format, determine if bitreverse is needed */
metadata->bitreverse = dsf_fmt_chunk.bitssample == 1;
return true;
@@ -285,7 +306,7 @@ dsf_stream_decode(struct decoder *decode
decoder_initialized(decoder, &audio_format, false, songtime);
if (!dsf_decode_chunk(decoder, is, metadata.channels,
- metadata.chunk_size,
+ chunk_size,
metadata.bitreverse))
return;
}
@@ -316,6 +337,10 @@ dsf_scan_stream(struct input_stream *is,
metadata.sample_rate;
tag_handler_invoke_duration(handler, handler_ctx, songtime);
+#ifdef HAVE_ID3TAG
+ /* Add available tags from the ID3 tag */
+ dsdlib_tag_id3(is, handler, handler_ctx, metadata.id3_offset);
+#endif
return true;
}
diff -uNrp mpd/src/tag_id3.c mpd-git-0.18-jk/src/tag_id3.c
--- mpd/src/tag_id3.c 2012-09-19 14:30:05.086747096 +0200
+++ mpd-git-0.18-jk/src/tag_id3.c 2012-09-19 14:39:39.206779200 +0200
@@ -343,7 +343,7 @@ tag_id3_import_ufid(struct id3_tag *id3_
}
}
-static void
+void
scan_id3_tag(struct id3_tag *tag,
const struct tag_handler *handler, void *handler_ctx)
{
diff -uNrp mpd/src/tag_id3.h mpd-git-0.18-jk/src/tag_id3.h
--- mpd/src/tag_id3.h 2012-09-19 14:30:05.086747096 +0200
+++ mpd-git-0.18-jk/src/tag_id3.h 2012-09-19 14:41:47.860786393 +0200
@@ -48,6 +48,13 @@ struct tag *tag_id3_import(struct id3_ta
struct id3_tag *
tag_id3_load(const char *path_fs, GError **error_r);
+/**
+ * Import all tags from the provided id3_tag *tag
+ */
+void
+scan_id3_tag(struct id3_tag *tag,
+ const struct tag_handler *handler, void *handler_ctx);
+
#else
static inline bool
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
