Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#12258 closed defect (worksforme)

AGS: Buffer overflow in AdLib driver at the start of Black Cauldron

Reported by: criezy Owned by: dreammaster
Priority: normal Component: Engine: AGS
Version: Keywords:
Cc: Game:


When starting Black Cauldron, when it reaches the title screen after the Disney logo, get a buffer overflow. It does not matter if you skip or not the Disney logo sound with Esc. Either way the bugger overflow occurs.

Here is the address-sanitiser report:

==90047==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000105353c58 at pc 0x0001050c4e0c bp 0x00016c0aa990 sp 0x00016c0aa988
READ of size 1 at 0x000105353c58 thread T8
    #0 0x1050c4e08 in MidiDriver_ADLIB::adlibPlayNote(int, int) adlib.cpp:1920
    #1 0x1050c7a5c in MidiDriver_ADLIB::adlibNoteOnEx(int, unsigned char, int) adlib.cpp:2282
    #2 0x1050c6bb0 in MidiDriver_ADLIB::mcKeyOn(AdLibVoice*, AdLibInstrument const*, unsigned char, unsigned char, AdLibInstrument const*, unsigned char) adlib.cpp:2078
    #3 0x1050bb0c8 in MidiDriver_ADLIB::partKeyOn(AdLibPart*, AdLibInstrument const*, unsigned char, unsigned char, AdLibInstrument const*, unsigned char) adlib.cpp:1956
    #4 0x1050bafe4 in AdLibPart::noteOn(unsigned char, unsigned char) adlib.cpp:1052
    #5 0x1050c1a7c in MidiDriver_ADLIB::send(signed char, unsigned int) adlib.cpp:1508
    #6 0x1050bacc4 in AdLibPart::send(unsigned int) adlib.cpp:1038
    #7 0x1042cbcd0 in AGS::Music::sendToChannel(unsigned char, unsigned int) music.cpp:63
    #8 0x1050f0a00 in Audio::MidiPlayer::send(unsigned int) midiplayer.cpp:111
    #9 0x1050e9040 in MidiParser::sendToDriver(unsigned int) midiparser.cpp:88
    #10 0x1050e7dec in MidiParser_SMF::sendToDriver(unsigned int) midiparser_smf.cpp:402
    #11 0x1050e9e38 in MidiParser::sendToDriver(unsigned char, unsigned char, unsigned char) midiparser.h:341
    #12 0x1050ec0a4 in MidiParser::processEvent(EventInfo const&, bool) midiparser.cpp:311
    #13 0x1050eaca8 in MidiParser::onTimer() midiparser.cpp:240
    #14 0x1050f11f0 in Audio::MidiPlayer::onTimer() midiplayer.cpp:157
    #15 0x1050f1008 in Audio::MidiPlayer::timerCallback(void*) midiplayer.cpp:147
    #16 0x1050c0fa8 in MidiDriver_ADLIB::onTimer() adlib.cpp:1628
 #17 0x1050ca928 in Common::Functor0Mem<void, MidiDriver_ADLIB>::operator()() const func.h:398
    #18 0x1050d840c in OPL::EmulatedOPL::readBuffer(short*, int) fmopl.cpp:337
    #19 0x1051b8d4c in Audio::CopyRateConverter<false, false>::flow(Audio::AudioStream&, short*, unsigned int, unsigned short, unsigned short) rate.cpp:315
    #20 0x1050f3824 in Audio::Channel::mix(short*, unsigned int) mixer.cpp:618
    #21 0x1050f3170 in Audio::MixerImpl::mixCallback(unsigned char*, unsigned int) mixer.cpp:293
    #22 0x104ced148 in SdlMixerManager::callbackHandler(unsigned char*, int) sdl-mixer.cpp:189
    #23 0x104ced050 in SdlMixerManager::sdlCallback(void*, unsigned char*, int) sdl-mixer.cpp:196
0x000105353c58 is located 8 bytes to the left of global variable 'Audio::MSADPCMAdaptationTable' defined in 'audio/decoders/adpcm.cpp:368:18' (0x105353c60) of size 64
0x000105353c58 is located 48 bytes to the right of global variable 'Audio::s_xaTable' defined in 'audio/decoders/adpcm.cpp:151:18' (0x105353c00) of size 40

The issue is not recent. I went as far back as commit 5e8e40d (Feb 7) and it still occurs.

Change History (5)

comment:1 by criezy, 3 years ago

And debugger output:

frame #5: 0x0000000100f18e0c scummvm`MidiDriver_ADLIB::adlibPlayNote(this=0x000000011a8ca100, channel=1, note=0) at adlib.cpp:1920:29
   1917		}
   1919		i = (notex << 3) + ((note >> 4) & 0x7);
-> 1920		adlibWrite(channel + 0xA0, g_noteFrequencies[i]);
   1921		adlibWrite(channel + 0xB0, oct | 0x20);
   1922	}
(lldb) p i
(int) $1 = 2040
(lldb) p sizeof(g_noteFrequencies)
(unsigned long) $2 = 144
(lldb) p note
(int) $3 = 0
(lldb) p notex
(byte) $4 = '?'
(lldb) p (int)notex
(int) $5 = 255

comment:2 by criezy, 3 years ago

The issue occurs when trying to play the music1.mid file. The file plays without error in VLC.

Changing the preferred audio device to use FluidSynth or Apple DLS Synth instead of the default also plays this properly. So the issue might be in the MidiDriver_ADLIB itself and not in AGS or the game data.

Maybe we just need to add a sanity check:

  • audio/adlib.cpp

    a b void MidiDriver_ADLIB::adlibPlayNote(int channel, int note) {  
    19171917       }
    19191919       i = (notex << 3) + ((note >> 4) & 0x7);
    1920        adlibWrite(channel + 0xA0, g_noteFrequencies[i]);
    1921        adlibWrite(channel + 0xB0, oct | 0x20);
     1920       if (i >= 0 && i < sizeof(g_noteFrequencies)) {
     1921               adlibWrite(channel + 0xA0, g_noteFrequencies[i]);
     1922               adlibWrite(channel + 0xB0, oct | 0x20);
     1923       }

Or should the case where note == 0 be handled before we get there, such as in AdLibPart::noteOn()?

comment:4 by dreammaster, 3 years ago

Owner: set to dreammaster
Resolution: worksforme
Status: newclosed

I've done multiple tests recently, and not been able to replicate any crash. Given the number of changes to the support code over time, and the complete replacement of the AGS codebase, it may be that the problem has simply been fixed at some point. I'm closing this for now, and we can re-open it again in the future if it's able to be replicated again.

comment:5 by criezy, 3 years ago

I just tested again, and I confirm that it works for me now as well.

Note: See TracTickets for help on using tickets.