Fix critical memory-safety and concurrency bugs found in code review

- handleController: bounds-check NRPN/CC controller ID before indexing
  last_midiCC_info[] (NRPN IDs can reach 16383, array is sized 1024)
- JaySynthMidiCC::add/remove: also reject negative controller IDs coming
  from patch/bank file data, not just out-of-range-high ones
- bankImportXml/loadPatchFromFile: null-check XML elements after
  getFirstChildElement()/XmlDocument::parse() before dereferencing, so
  malformed/truncated patch or bank files no longer crash on load
- loadBankFromFile/loadPatchFromFile: validate the loaded file is large
  enough for the fxBank/fxProgram header, and that the embedded chunk
  size fits within the file, before reading through the raw pointer
- processBlock/JaySynth::renderNextBlock: host blocks larger than
  SYNTH_MAX_BUFSIZE no longer overflow the fixed-size stack work buffers
  or deadlock the render worker threads; oversized blocks are now
  rendered in multiple SYNTH_MAX_BUFSIZE-sized passes (pending MIDI
  events are correctly held over between passes)
- JaySynth constructor: guard against createInputStream() returning null
  when the wavetable file can't be opened
- JaySynth::setParameter/setControl/setPerVoiceControl/setParam/
  ClearControls: take the synth's lock, matching renderNextBlock and the
  note/controller handlers, to close a data race between parameter
  changes (UI/host automation) and audio-thread rendering

Verified with a full rebuild (make) producing JaySynth.so with no new
warnings or errors.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ

src/plug/JaySynth.cpp

@@ -53,7 +53,8 @@ JaySynth::JaySynth (int num_voices, String pathToWaves)
 	{
 		File wavesFile = File(pathToWaves);
 	    ScopedPointer<FileInputStream>pIn = wavesFile.createInputStream();
-		pIn->read(synth_common.vco.wt.wave_rawdata, sizeof(synth_common.vco.wt.wave_rawdata));
+		if (pIn != NULL)
+			pIn->read(synth_common.vco.wt.wave_rawdata, sizeof(synth_common.vco.wt.wave_rawdata));
 	}
 
 	max_num_voices = num_voices;
@@ -379,6 +380,8 @@ void JaySynth::humanizeVarianceChanged_ENV(void)
 //==============================================================================
 void JaySynth::ClearControls(void)
 {
+	const ScopedLock sl (lock);
+
 	memset(controls, 0, SYNTH_NUM_PARAMS*sizeof(synth_float_t));
 }
 
@@ -428,6 +431,8 @@ synth_float_t JaySynth::getParameter(int paramID)
 
 void JaySynth::setParameter(int paramID, synth_float_t param)
 {
+	const ScopedLock sl (lock);
+
 	params[paramID] = param;
 	setParam(paramID, -1);
 
@@ -437,6 +442,8 @@ void JaySynth::setParameter(int paramID, synth_float_t param)
 
 void JaySynth::setControl(int paramID, synth_float_t param)
 {
+	const ScopedLock sl (lock);
+
 	controls[paramID] = param;
 	setParam(paramID, -1);
 
@@ -446,12 +453,16 @@ void JaySynth::setControl(int paramID, synth_float_t param)
 
 void JaySynth::setPerVoiceControl(int voice, int channel, int paramID, synth_float_t param)
 {
+	const ScopedLock sl (lock);
+
 	pPer_voice_controls[voice].ctrl[channel][paramID] = param;
 	setParam(paramID, voice);
 }
 
 void JaySynth::setParam(int paramID, int voice)
 {
+	const ScopedLock sl (lock);
+
 	int i, j, voice_param_type, voice_min, voice_max;
 	synth_float_t param;
 
@@ -898,6 +909,9 @@ void JaySynth::handlePitchWheel (const int midiChannel, const int wheelValue)
 
 void JaySynth::handleController (const midiCC_info_t &midiCC_info)
 {
+	if (midiCC_info.ID < 0 || midiCC_info.ID >= NUM_MIDI_CONTROLLERS)
+		return;
+
 	last_midiCC_info[midiCC_info.ID] = midiCC_info;
     listeners.call (&JaySynthListener::synthChanged, SYNTH_CHANGED_MIDICC, (midiCC_info_t*)&midiCC_info);
 
@@ -990,14 +1004,26 @@ void JaySynth::renderNextBlock (AudioSampleBuffer& outputBuffer,
     midiIterator.setNextSamplePosition (startSample);
     MidiMessage m (0xf4, 0.0);
 
+    bool havePendingEvent = false;
+    int midiEventPos = 0;
+
     while (numSamples > 0)
     {
-        int midiEventPos;
-        const bool useEvent = midiIterator.getNextEvent (m, midiEventPos)
+        if (!havePendingEvent)
+            havePendingEvent = midiIterator.getNextEvent (m, midiEventPos);
+
+        const bool useEvent = havePendingEvent
                                 && midiEventPos < startSample + numSamples;
 
-        const int numThisTime = useEvent ? midiEventPos - startSample
-                                         : numSamples;
+        int numThisTime = useEvent ? midiEventPos - startSample
+                                   : numSamples;
+
+        // The DSP core's per-block work buffers are fixed at SYNTH_MAX_BUFSIZE
+        // samples, so render oversized ranges in multiple passes rather than
+        // overrunning them. The pending midi event (if any) stays queued until
+        // we actually reach its sample position.
+        if (numThisTime > SYNTH_MAX_BUFSIZE)
+            numThisTime = SYNTH_MAX_BUFSIZE;
 
         if (numThisTime > 0)
         {
@@ -1035,9 +1061,12 @@ void JaySynth::renderNextBlock (AudioSampleBuffer& outputBuffer,
 				}
 			}
 		}
-        if (useEvent)
+        // Only fire the pending event once we've actually rendered up to its
+        // sample position (numThisTime may have been clamped short of it above).
+        if (useEvent && numThisTime == midiEventPos - startSample)
 		{
 			handleMidiEvent (m);
+			havePendingEvent = false;
 		}
 
         startSample += numThisTime;

src/plug/JaySynthMidiCC.cpp

@@ -432,7 +432,7 @@ int JaySynthMidiCC::import_midi_cc_legacy(XmlElement const *pXML_MidiCc)
 
 void JaySynthMidiCC::add(midiCC_container_t *pObj, int controllerID)
 {
-	if (controllerID >= NUM_MIDI_CONTROLLERS)
+	if (controllerID < 0 || controllerID >= NUM_MIDI_CONTROLLERS)
 		return;
 
 	remove(pObj, pObj->controllerID);
@@ -448,7 +448,7 @@ void JaySynthMidiCC::add(midiCC_container_t *pObj, int controllerID)
 
 void JaySynthMidiCC::remove(midiCC_container_t *pObj, int controllerID)
 {
-	if (controllerID >= NUM_MIDI_CONTROLLERS)
+	if (controllerID < 0 || controllerID >= NUM_MIDI_CONTROLLERS)
 		return;
 	
 	pObj->isAssigned = 0;

src/plug/PluginProcessor.cpp

@@ -8,6 +8,7 @@
   ==============================================================================
 */
 #include <math.h>
+#include <cstddef>
 
 #include "PluginProcessor.h"
 #include "JaySynthAudioProcessorEditor.h"
@@ -546,18 +547,16 @@ void JaySynthAudioProcessor::releaseResources()
 void JaySynthAudioProcessor::processBlock (AudioSampleBuffer& buffer, MidiBuffer& midiMessages)
 {
 	const int numSamples = buffer.getNumSamples();
-    int i;
 	float *buf1 = buffer.getWritePointer (0, 0);
 	float *buf2 = buffer.getWritePointer (1, 0);
-	synth_float_t buf[2][SYNTH_MAX_BUFSIZE];
 	float synth_gain;
 
 	AudioPlayHead::CurrentPositionInfo posInfo;
 	getPlayHead()->getCurrentPosition(posInfo);
-	
+
 	// Parse midi buffer and add midi clock messages
 	m_JK_MidiClock.generateMidiclock(posInfo, &midiMessages, numSamples, getSampleRate());
-	
+
 	// Now pass any incoming midi messages to our keyboard state object, and let it
     // add messages to the buffer if the user is clicking on the on-screen keys
     keyboardState.processNextMidiBuffer (midiMessages, 0, numSamples, true);
@@ -566,78 +565,88 @@ void JaySynthAudioProcessor::processBlock (AudioSampleBuffer& buffer, MidiBuffer
 	buffer.clear (0, 0, numSamples);
 	buffer.clear (1, 0, numSamples);
 
-	// and now get the synth to process these midi events and generate its output.
-	// Render individual voices
-	pSynth->renderNextBlock (buffer, midiMessages, 0, numSamples);
-
 	// apply master volume
 	synth_gain = (float)pSynth->getVolume();
 
-	for (i=0; i <numSamples; i++)
-		buf[0][i] = (synth_float_t)buf1[i] * synth_gain;
-
-	for (i=0; i <numSamples; i++)
-		buf[1][i] = (synth_float_t)buf2[i] * synth_gain;
-
-	m_limiter_active = 0;
-
 	synth_float_t master_gain = 0.5;
-	
-#ifdef WITH_LIMITER		
-	synth_float_t limiter_env[SYNTH_MAX_BUFSIZE];
-	synth_float_t limiter_gain[SYNTH_MAX_BUFSIZE];
-	synth_float_t x, limiter_ar, limiter_af, limiter_threshold;
 
-	// Limiter
-	limiter_ar = 5.0/(SYNTH_LIMITER_RISE_TIME*m_fs);
-	limiter_af = 5.0/(SYNTH_LIMITER_FALL_TIME*m_fs);
-	limiter_threshold = pow((synth_float_t)10.0, (synth_float_t)SYNTH_LIMITER_THRESHOLD/20);
-
-	for (i=0; i <numSamples; i++)
+	// The DSP core (and this function's own work buffers) can only render up to
+	// SYNTH_MAX_BUFSIZE samples per call, so process oversized host blocks in
+	// multiple chunks instead of overflowing the fixed-size stack buffers below.
+	for (int chunkStart = 0; chunkStart < numSamples; chunkStart += SYNTH_MAX_BUFSIZE)
 	{
-		x = jsy_max(fabs(buf[0][i]), fabs(buf[1][i]));
-		if (x > m_limiter_env)
+		const int chunkLen = jsy_min(numSamples - chunkStart, (int)SYNTH_MAX_BUFSIZE);
+		synth_float_t buf[2][SYNTH_MAX_BUFSIZE];
+		int i;
+
+		// and now get the synth to process these midi events and generate its output.
+		// Render individual voices
+		pSynth->renderNextBlock (buffer, midiMessages, chunkStart, chunkLen);
+
+		for (i=0; i <chunkLen; i++)
+			buf[0][i] = (synth_float_t)buf1[chunkStart + i] * synth_gain;
+
+		for (i=0; i <chunkLen; i++)
+			buf[1][i] = (synth_float_t)buf2[chunkStart + i] * synth_gain;
+
+		m_limiter_active = 0;
+
+#ifdef WITH_LIMITER
+		synth_float_t limiter_env[SYNTH_MAX_BUFSIZE];
+		synth_float_t limiter_gain[SYNTH_MAX_BUFSIZE];
+		synth_float_t x, limiter_ar, limiter_af, limiter_threshold;
+
+		// Limiter
+		limiter_ar = 5.0/(SYNTH_LIMITER_RISE_TIME*m_fs);
+		limiter_af = 5.0/(SYNTH_LIMITER_FALL_TIME*m_fs);
+		limiter_threshold = pow((synth_float_t)10.0, (synth_float_t)SYNTH_LIMITER_THRESHOLD/20);
+
+		for (i=0; i <chunkLen; i++)
 		{
-			m_limiter_env = (1.0-limiter_ar)*m_limiter_env + limiter_ar*x;
-		}
-		else
-		{
-			m_limiter_env = (1.0-limiter_af)*m_limiter_env;
+			x = jsy_max(fabs(buf[0][i]), fabs(buf[1][i]));
+			if (x > m_limiter_env)
+			{
+				m_limiter_env = (1.0-limiter_ar)*m_limiter_env + limiter_ar*x;
+			}
+			else
+			{
+				m_limiter_env = (1.0-limiter_af)*m_limiter_env;
+			}
+
+			limiter_env[i] = m_limiter_env;
 		}
 
-		limiter_env[i] = m_limiter_env;
-	}
-
-	for (i=0; i <numSamples; i++)
-	{
-		limiter_gain[i] = jsy_min(1.0, 1.0/(limiter_env[i] + 1.0e-6));
-		if (limiter_gain[i] < 1.0)
+		for (i=0; i <chunkLen; i++)
 		{
-			m_limiter_active = 1;
-			break;
+			limiter_gain[i] = jsy_min(1.0, 1.0/(limiter_env[i] + 1.0e-6));
+			if (limiter_gain[i] < 1.0)
+			{
+				m_limiter_active = 1;
+				break;
+			}
 		}
-	}
 
-	for (i=0; i <numSamples; i++)
-	{
-		buf1[i] = (float)(master_gain*buf[0][i]*limiter_gain[i]);
-	}
+		for (i=0; i <chunkLen; i++)
+		{
+			buf1[chunkStart + i] = (float)(master_gain*buf[0][i]*limiter_gain[i]);
+		}
 
-	for (i=0; i <numSamples; i++)
-	{
-		buf2[i] = (float)(master_gain*buf[1][i]*limiter_gain[i]);
-	}
+		for (i=0; i <chunkLen; i++)
+		{
+			buf2[chunkStart + i] = (float)(master_gain*buf[1][i]*limiter_gain[i]);
+		}
 #else
-	for (i=0; i <numSamples; i++)
-	{
-		buf1[i] = (float)(master_gain*buf[0][i]);
-	}
+		for (i=0; i <chunkLen; i++)
+		{
+			buf1[chunkStart + i] = (float)(master_gain*buf[0][i]);
+		}
 
-	for (i=0; i <numSamples; i++)
-	{
-		buf2[i] = (float)(master_gain*buf[1][i]);
-	}
+		for (i=0; i <chunkLen; i++)
+		{
+			buf2[chunkStart + i] = (float)(master_gain*buf[1][i]);
+		}
 #endif
+	}
 
     // In case we have more outputs than inputs, we'll clear any output
     // channels that didn't contain input data, (because these aren't
@@ -838,6 +847,11 @@ void JaySynthAudioProcessor::loadBankFromFile (const File &file)
 	{
 		MemoryBlock mem;
 		file.loadFileAsData(mem);
+
+		const size_t headerSize = offsetof(fxBank, content) + sizeof(VstInt32);
+		if (mem.getSize() < headerSize)
+			return;
+
 		fxBank *bank = (fxBank*)mem.getData();
 		if (String((const char*)&bank->chunkMagic) == String(cMagic))
 		{
@@ -846,6 +860,9 @@ void JaySynthAudioProcessor::loadBankFromFile (const File &file)
 			if (is_opaque)
 			{
 				uint32_t size = ByteOrder::swap((uint32_t)bank->content.data.size);
+				const size_t chunkOffset = offsetof(fxBank, content.data.chunk);
+				if (chunkOffset > mem.getSize() || (size_t)size > mem.getSize() - chunkOffset)
+					return;
 				setStateInformation(bank->content.data.chunk, (int)size);
 			}
 		}
@@ -864,7 +881,7 @@ void JaySynthAudioProcessor::bankImportXml (const XmlElement *xml)
 		if (xml->hasTagName ("JSynth"))
 		{
 			XmlElement *xml2 = xml->getFirstChildElement();
-			if (xml2->hasTagName ("Bank"))
+			if (xml2 != 0 && xml2->hasTagName ("Bank"))
 			{
 				XmlElement *xml3 = xml2->getFirstChildElement();
 				bankDecodeXml(xml3);
@@ -876,8 +893,9 @@ void JaySynthAudioProcessor::bankImportXml (const XmlElement *xml)
 			if (xml->hasTagName ("JSYNTH_BANK"))
 			{
 				XmlElement *xml2 = xml->getFirstChildElement();
-				bankDecodeXml_legacy(*xml2);
-				
+				if (xml2 != 0)
+					bankDecodeXml_legacy(*xml2);
+
 			}
 		}
    		setCurrentProgram(getCurrentProgram());
@@ -934,6 +952,11 @@ void JaySynthAudioProcessor::loadPatchFromFile (const File &file, int index)
 	{
 		MemoryBlock mem;
 		file.loadFileAsData(mem);
+
+		const size_t headerSize = offsetof(fxProgram, content) + sizeof(VstInt32);
+		if (mem.getSize() < headerSize)
+			return;
+
 		fxProgram *program = (fxProgram*)mem.getData();
 		if (String((const char*)&program->chunkMagic) == String(cMagic))
 		{
@@ -942,6 +965,9 @@ void JaySynthAudioProcessor::loadPatchFromFile (const File &file, int index)
 			if (is_opaque)
 			{
 				uint32_t size = ByteOrder::swap((uint32_t)program->content.data.size);
+				const size_t chunkOffset = offsetof(fxProgram, content.data.chunk);
+				if (chunkOffset > mem.getSize() || (size_t)size > mem.getSize() - chunkOffset)
+					return;
 				setCurrentProgramStateInformation(program->content.data.chunk, (int)size);
 			}
 		}
@@ -949,7 +975,7 @@ void JaySynthAudioProcessor::loadPatchFromFile (const File &file, int index)
 	else if (String(".xmp") == ext)
 	{
 		ScopedPointer<XmlElement> xml = XmlDocument::parse(file);
-		if (xml->hasTagName ("JSynth"))
+		if (xml != 0 && xml->hasTagName ("JSynth"))
 		{
 			patchImportXml(xml, currpatch);
 		}