xine-lib and ocert-2008-008

[Thomas Viehmann <tv () beamnet de>]

[resending this with hopefully less broken CC, apologies]

Hi,

I am not quite sure whether I can agree with Will Drewry's analysis[1]
accompanying ocert advisory 2008-008[1]. Looking at item 1A, which Will
says is fixed in 1.1.5, attached .mov seems to fit the case description
and will still corrupt the memory when viewed e.g. in gxine. xine-lib
with the attached patch seems to be more successful in preventing the
attach (note that the file is more tuned to be small than to be a valid
.mov, but the same works by including the bad meta in an otherwise good
file). Note that xine_xmalloc is specifically designed to allocate
memory when passed size 0. Upstream seems to move away from it, but...
As Will notices, demux-qt.c has loads of unfixed problems.

If anyone cares to go over the xine-lib issues (primarily the unfixed
ones from Will's section 3), I'd much appreciate a CC. In order to make
the analysis and verification more, I would also be interested in the
test cases mentioned in the advisory.

Kind regards

T.

1. http://www.ocert.org/analysis/2008-008/analysis.txt
2. http://www.ocert.org/advisories/ocert-2008-008.html
-- 
Thomas Viehmann, http://thomas.viehmann.net/

--- xine-lib-1.1.14.orig/src/demuxers/demux_qt.c
+++ xine-lib-1.1.14/src/demuxers/demux_qt.c
@@ -739,49 +739,49 @@
     if (current_atom == ART_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->artist = xine_xmalloc(string_size);
-      if (info->artist) {
+      if (string_size && info->artist) {
         strncpy(info->artist, &meta_atom[i + 20], string_size - 1);
         info->artist[string_size - 1] = 0;
       }
     } else if (current_atom == NAM_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->name = xine_xmalloc(string_size);
-      if (info->name) {
+      if (string_size && info->name) {
         strncpy(info->name, &meta_atom[i + 20], string_size - 1);
         info->name[string_size - 1] = 0;
       }
     } else if (current_atom == ALB_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->album = xine_xmalloc(string_size);
-      if (info->album) {
+      if (string_size && info->album) {
         strncpy(info->album, &meta_atom[i + 20], string_size - 1);
         info->album[string_size - 1] = 0;
       }
     } else if (current_atom == GEN_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->genre = xine_xmalloc(string_size);
-      if (info->genre) {
+      if (string_size && info->genre) {
         strncpy(info->genre, &meta_atom[i + 20], string_size - 1);
         info->genre[string_size - 1] = 0;
       }
     } else if (current_atom == TOO_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->comment = xine_xmalloc(string_size);
-      if (info->comment) {
+      if (string_size && info->comment) {
         strncpy(info->comment, &meta_atom[i + 20], string_size - 1);
         info->comment[string_size - 1] = 0;
       }
     } else if (current_atom == WRT_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->composer = xine_xmalloc(string_size);
-      if (info->composer) {
+      if (string_size && info->composer) {
         strncpy(info->composer, &meta_atom[i + 20], string_size - 1);
         info->composer[string_size - 1] = 0;
       }
     } else if (current_atom == DAY_ATOM) {
       string_size = _X_BE_32(&meta_atom[i + 4]) - 16 + 1;
       info->year = xine_xmalloc(string_size);
-      if (info->year) {
+      if (string_size && info->year) {
         strncpy(info->year, &meta_atom[i + 20], string_size - 1);
         info->year[string_size - 1] = 0;
       }