Processed: Re: Bug#890023: abcmidi fails autopkg tests on 32bit architectures
Processing control commands: > tags -1 pending Bug #890023 [src:abcmidi] abcmidi fails autopkg tests on 32bit architectures Added tag(s) pending. -- 890023: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890023 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#890023: abcmidi fails autopkg tests on 32bit architectures
Control: tags -1 pending
Hi James,
As usual, thanks for your help in tracking this down. I had noticed that
abcmidi was stuck in ubuntu -proposed, but I had not had time to
investigate.
As the changes seem to have been about fixing a windows problem, I am
sure the patch will be fine for Debian/Ubuntu. Seymour (upstream) will
no doubt consider applying the patch, or something better if it causes a
regression on Windows.
I hope to apply the patch in Debian, and upload with the latest upstream
release sometime this week.
Regards,
Ross
On 02/12/2018 02:28 PM, James Cowgill wrote:
> Control: tags -1 patch
>
> Hi,
>
> On 10/02/18 08:52, Matthias Klose wrote:
>> Package: src:abcmidi
>> Version: 20180125-1
>> Severity: serious
>> Tags: sid buster
>>
>> abcmidi fails autopkg tests on 32bit architectures, midi2abc crashing. Is
>> there
>> a reason why the tests are not run at build time?
>>
>>
>> MIDI to ABC conversion test
>>
>> Convert the araber47.mid file back to abc
>> Aborted (core dumped)
>> autopkgtest [14:08:29]: test conversions: ---]
>> autopkgtest [14:08:33]: test conversions: - - - - - - - - - - results - - -
>> - -
>> - - - - -
>> conversions FAIL non-zero exit status 134
>> autopkgtest [14:08:33]: test conversions: - - - - - - - - - - stderr - - -
>> - -
>> - - - - -
>> *** Error in `midi2abc': free(): invalid pointer: 0x00c43f28 ***
>> Aborted (core dumped)
> Caused by a buffer overflow in midi2abc.c:329
>> char* addstring(s)
>> /* create space for string and store it in memory */
>> char* s;
>> {
>> char* p;
>>
>> p = (char*) checkmalloc(strlen(s)+1);
>> strncpy(p, s,strlen(s)+2); /* [SS] 2017-08-30 */
>> return(p);
>> }
> strncpy writes to exactly n bytes to the output buffer, so the call will
> always overflow the buffer allocated one line above by 1 byte.
>
> Attached patch fixes this. I think using strcpy is safe here because the
> size of the buffer allocated is always greater than the string length.
>
> I think the comment on that line refers to this (from doc/CHANGES):
>> August 30 2017
>>
>> Midi2abc - The metatext string is not terminated with a 0 and
>> as a result can contain random junk, in particular on the Windows
>> operating system. Fix in midifile.c, the Msgbuff is initialized to
>> 0 when it is allocated.
> Some old code I found shows the code originally used strcpy. I'm not I
> understand how using strncpy was supposed to fix this. I've copied
> upstream who might be able to shed some light on this.
>
> Thanks,
> James
Bug#890023: abcmidi fails autopkg tests on 32bit architectures
Control: tags -1 patch
Hi,
On 10/02/18 08:52, Matthias Klose wrote:
> Package: src:abcmidi
> Version: 20180125-1
> Severity: serious
> Tags: sid buster
>
> abcmidi fails autopkg tests on 32bit architectures, midi2abc crashing. Is
> there
> a reason why the tests are not run at build time?
>
>
> MIDI to ABC conversion test
>
> Convert the araber47.mid file back to abc
> Aborted (core dumped)
> autopkgtest [14:08:29]: test conversions: ---]
> autopkgtest [14:08:33]: test conversions: - - - - - - - - - - results - - -
> - -
> - - - - -
> conversions FAIL non-zero exit status 134
> autopkgtest [14:08:33]: test conversions: - - - - - - - - - - stderr - - - -
> -
> - - - - -
> *** Error in `midi2abc': free(): invalid pointer: 0x00c43f28 ***
> Aborted (core dumped)
Caused by a buffer overflow in midi2abc.c:329
> char* addstring(s)
> /* create space for string and store it in memory */
> char* s;
> {
> char* p;
>
> p = (char*) checkmalloc(strlen(s)+1);
> strncpy(p, s,strlen(s)+2); /* [SS] 2017-08-30 */
> return(p);
> }
strncpy writes to exactly n bytes to the output buffer, so the call will
always overflow the buffer allocated one line above by 1 byte.
Attached patch fixes this. I think using strcpy is safe here because the
size of the buffer allocated is always greater than the string length.
I think the comment on that line refers to this (from doc/CHANGES):
> August 30 2017
>
> Midi2abc - The metatext string is not terminated with a 0 and
> as a result can contain random junk, in particular on the Windows
> operating system. Fix in midifile.c, the Msgbuff is initialized to
> 0 when it is allocated.
Some old code I found shows the code originally used strcpy. I'm not I
understand how using strncpy was supposed to fix this. I've copied
upstream who might be able to shed some light on this.
Thanks,
James
--- a/midi2abc.c
+++ b/midi2abc.c
@@ -333,7 +333,7 @@ char* s;
char* p;
p = (char*) checkmalloc(strlen(s)+1);
- strncpy(p, s,strlen(s)+2); /* [SS] 2017-08-30 */
+ strcpy(p, s);
return(p);
}
signature.asc
Description: OpenPGP digital signature
Processed: Re: Bug#890023: abcmidi fails autopkg tests on 32bit architectures
Processing control commands: > tags -1 patch Bug #890023 [src:abcmidi] abcmidi fails autopkg tests on 32bit architectures Added tag(s) patch. -- 890023: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890023 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#890023: abcmidi fails autopkg tests on 32bit architectures
Package: src:abcmidi Version: 20180125-1 Severity: serious Tags: sid buster abcmidi fails autopkg tests on 32bit architectures, midi2abc crashing. Is there a reason why the tests are not run at build time? MIDI to ABC conversion test Convert the araber47.mid file back to abc Aborted (core dumped) autopkgtest [14:08:29]: test conversions: ---] autopkgtest [14:08:33]: test conversions: - - - - - - - - - - results - - - - - - - - - - conversions FAIL non-zero exit status 134 autopkgtest [14:08:33]: test conversions: - - - - - - - - - - stderr - - - - - - - - - - *** Error in `midi2abc': free(): invalid pointer: 0x00c43f28 *** Aborted (core dumped) see on http://autopkgtest.ubuntu.com/packages/a/abcmidi/bionic/i386 http://autopkgtest.ubuntu.com/packages/a/abcmidi/bionic/armhf
