Bug#1002892: abcmidi: Stack based buffer overflow in the karaokestarttrack function used by abc2midi

[Kolja Grassmann <koljagrassmann@mailbox.org>]

Package: abcmidi
Version: 20211212-1
Severity: important
Tags: security

Dear Maintainer,

There is a stack based buffer overflow in the karaokestarttrack function from the genmidi.c file, which is used by the abc2midi application. The responsible code looks as follows:


static void karaokestarttrack (track)
int track;
/* header information for karaoke track based on w: fields */
{
  int j;
  int done;
  char atitle[200];
[...]
 while ((j < notes) && (done > 0))
  {
     j = j+1;
     if (feature[j] == TITLE) {
        if (track != 2)
           mf_write_meta_event(0L, sequence_name, atext[pitch[j]], strlen (atext[pitch[j]]));
        strcpy(atitle+2, atext[pitch[j]]);
        text_data(atitle);
        done--;
     }
     if (feature[j] == COMPOSER) {
        strcpy(atitle+2, atext[pitch[j]]);
        text_data(atitle);
        done--;
     }     
     if (feature[j] == COPYRIGHT) {
        strcpy(atitle+2, atext[pitch[j]]);
        text_data(atitle);
        done--;
     }
  }
}

In the while loop strcpy is used to copy data into the atitle buffer, which is of size 200, without any length check on the data. Therefore it is possible to copy over 200 into the atitle buffer overwriting othere data on the stack. I wrote the following poc script in python to test this:

#!/bin/python3

filecontent = b"""X:
T:""" + b"A" * 400 +  b"""
w:
K:D"""

f = open("poc.abc", "wb")
f.write(filecontent)
f.close()

The script generates a poc.abc file. Using abc2midi to convert the generated file leads to a memory corruption issue:
$ abc2midi poc.abc -o /dev/null
4.64 December 12 2021 abc2midi
Error in line-char 0-0 : Missing Number
Error in line-char 3-0 : missplaced w: field. w: field ignored
Warning in line-char 4-0 : No M: in header, using default
writing MIDI file /dev/null
*** buffer overflow detected ***: terminated
Aborted

Locally I fixed the issue by using strncpy instead of strcpy as follows:

static void karaokestarttrack (track)
int track;
/* header information for karaoke track based on w: fields */
{
  int j;
  int done;
  char atitle[200];
[...]
  while ((j < notes) && (done > 0))
  {
     j = j+1;
     if (feature[j] == TITLE) {
        if (track != 2)
           mf_write_meta_event(0L, sequence_name, atext[pitch[j]], strlen (atext[pitch[j]]));
        strncpy(atitle+2, atext[pitch[j]], 197);
        text_data(atitle);
        done--;
     }
     if (feature[j] == COMPOSER) {
        strncpy(atitle+2, atext[pitch[j]], 197);
        text_data(atitle);
        done--;
     }     
     if (feature[j] == COPYRIGHT) {
        strncpy(atitle+2, atext[pitch[j]], 197);
        text_data(atitle);
        done--;
     }
  }
}

This seemed to fix the buffer overflow for me. However if over 200 bytes of data are allowed in this location a different solution might be needed.

Best regards
Kolja

-- System Information:
Debian Release: 11.0
   APT prefers stable-updates
   APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-8-amd64 (SMP w/4 CPU threads)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages abcmidi depends on:
ii   libc6   2.31-13+deb11u2

abcmidi recommends no packages.

Versions of packages abcmidi suggests:
pn   abcm2ps                               <none>
ii   evince [postscript-viewer]   3.38.2-1
pn   timidity | pmidi                  <none>

-- no debconf information