Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions framework/vst/internal/vstcomponenthandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,30 @@ Notification VstComponentHandler::pluginParamsChanged() const
return m_paramsChangedNotify;
}

void VstComponentHandler::setSuppressNotify(bool suppress)
{
m_advancedHandler->setSuppressNotify(suppress);
}

Steinberg::tresult VstComponentHandler::beginEdit(Steinberg::Vst::ParamID /*id*/)
{
return Steinberg::kResultOk;
}

Steinberg::tresult VstComponentHandler::performEdit(Steinberg::Vst::ParamID id, Steinberg::Vst::ParamValue valueNormalized)
{
m_paramChanged.send(std::move(id), std::move(valueNormalized));
if (!m_advancedHandler->suppressNotify()) {
m_paramChanged.send(id, valueNormalized);
}

return Steinberg::kResultOk;
}

Steinberg::tresult VstComponentHandler::endEdit(Steinberg::Vst::ParamID /*id*/)
{
m_paramsChangedNotify.notify();
if (!m_advancedHandler->suppressNotify()) {
m_paramsChangedNotify.notify();
}

return Steinberg::kResultOk;
}
Expand All @@ -84,9 +93,21 @@ VstAdvancedHandler::VstAdvancedHandler(async::Notification notifier)
{
}

void VstAdvancedHandler::setSuppressNotify(bool suppress)
{
m_suppressNotify.store(suppress, std::memory_order_relaxed);
}

bool VstAdvancedHandler::suppressNotify() const
{
return m_suppressNotify.load(std::memory_order_relaxed);
}

Steinberg::tresult VstAdvancedHandler::setDirty(Steinberg::TBool /*state*/)
{
m_paramsChanged.notify();
if (!suppressNotify()) {
m_paramsChanged.notify();
}

return Steinberg::kResultOk;
}
Expand All @@ -103,7 +124,9 @@ Steinberg::tresult VstAdvancedHandler::startGroupEdit()

Steinberg::tresult VstAdvancedHandler::finishGroupEdit()
{
m_paramsChanged.notify();
if (!suppressNotify()) {
m_paramsChanged.notify();
}

return Steinberg::kResultOk;
}
11 changes: 11 additions & 0 deletions framework/vst/internal/vstcomponenthandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#ifndef MUSE_VST_VSTCOMPONENTHANDLER_H
#define MUSE_VST_VSTCOMPONENTHANDLER_H

#include <atomic>

#include "async/notification.h"
#include "async/channel.h"

Expand All @@ -35,12 +37,17 @@ class VstAdvancedHandler : public IAdvancedComponentHandler
VstAdvancedHandler(async::Notification notifier);
virtual ~VstAdvancedHandler() = default;

void setSuppressNotify(bool suppress);
bool suppressNotify() const;

Steinberg::tresult setDirty(Steinberg::TBool state) override;
Steinberg::tresult requestOpenEditor(Steinberg::FIDString name) override;
Steinberg::tresult startGroupEdit() override;
Steinberg::tresult finishGroupEdit() override;

private:
async::Notification m_paramsChanged;
std::atomic_bool m_suppressNotify = false;
};

class VstComponentHandler : public IComponentHandler
Expand All @@ -53,6 +60,10 @@ class VstComponentHandler : public IComponentHandler
async::Channel<PluginParamId, PluginParamValue> pluginParamChanged() const;
async::Notification pluginParamsChanged() const;

// Suppress pluginParamsChanged() while applying config programmatically,
// to avoid triggering a rescan of the state we are currently writing
void setSuppressNotify(bool suppress);

private:
Steinberg::tresult beginEdit(Steinberg::Vst::ParamID id) override;
Steinberg::tresult performEdit(Steinberg::Vst::ParamID id, Steinberg::Vst::ParamValue valueNormalized) override;
Expand Down
182 changes: 79 additions & 103 deletions framework/vst/internal/vstplugininstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ static const std::string_view CONTROLLER_STATE_KEY = "controllerState";

static VstPluginInstanceId s_lastId = 0;

static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize)
{
if (strSize == 0) {
return;
}

buffer.setSize(0);
buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
Comment on lines +42 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify all state-buffer writes that narrow size_t to Steinberg::int32.
# Expected: Any cast to Steinberg::int32 for stream byte counts is guarded by an upper-bound check.

rg -n -C4 'static_cast<Steinberg::int32>\(|\.write\(.*Steinberg::int32|stateBufferFromString' --iglob '*.cpp' --iglob '*.h'

Repository: musescore/muse_framework

Length of output: 3745


🏁 Script executed:

#!/bin/bash
# Check the Steinberg type definition and VST API expectations
rg -n 'int32|IBStream' --type h framework/ | head -30

Repository: musescore/muse_framework

Length of output: 3215


🏁 Script executed:

#!/bin/bash
# Look for any existing size validations on state before stateBufferFromString calls
rg -B10 'stateBufferFromString' framework/vst/internal/vstplugininstance.cpp | grep -i 'size\|max\|limit\|check'

Repository: musescore/muse_framework

Length of output: 305


🏁 Script executed:

#!/bin/bash
# Check if there are size limits defined elsewhere in VST plugin code
rg -n 'MAX.*SIZE|SIZE.*MAX|MAX.*BUFFER|numeric_limits' framework/vst/

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check Steinberg type definitions in the codebase
find framework -name "*.h" -type f | xargs grep -l "Steinberg.*int32\|namespace Steinberg" | head -5

Repository: musescore/muse_framework

Length of output: 157


🏁 Script executed:

#!/bin/bash
# Look for Steinberg header includes to understand the type
rg -n 'include.*Steinberg|Steinberg::int32' framework/vst/internal/vstplugininstance.cpp | head -10

Repository: musescore/muse_framework

Length of output: 147


🏁 Script executed:

#!/bin/bash
# Check the complete context around the write() method calls to see full signature
cat -n framework/vst/internal/vstplugininstance.cpp | sed -n '42,51p'

Repository: musescore/muse_framework

Length of output: 446


🏁 Script executed:

#!/bin/bash
# Verify if VstMemoryStream or IBStream has any documented size limits
rg -n 'class.*VstMemoryStream|class.*IBStream' framework/vst/ -A10 | head -40

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check the full context of both stateBufferFromString call sites
cat -n framework/vst/internal/vstplugininstance.cpp | sed -n '280,305p'

Repository: musescore/muse_framework

Length of output: 1368


🏁 Script executed:

#!/bin/bash
# Check where componentState and controllerState come from - are they from config maps?
rg -B20 'componentState.*config.end\|controllerState.*config.end' framework/vst/internal/vstplugininstance.cpp | head -50

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Look for any deserialization or config loading that might produce oversized state
rg -n 'readConfig|loadState|setState' framework/vst/internal/vstplugininstance.cpp | head -20

Repository: musescore/muse_framework

Length of output: 224


🏁 Script executed:

#!/bin/bash
# Check if there are any size limits or validations on state blobs elsewhere
rg -n 'maxSize|MAX_SIZE|max.*state|state.*max' framework/vst/ -i

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Verify what std::string::size() typically returns and whether it could exceed INT32_MAX
# Also check if there's any pruning/truncation of config states before loading
rg -B5 'second.*size\(\)' framework/vst/internal/vstplugininstance.cpp | head -30

Repository: musescore/muse_framework

Length of output: 645


🏁 Script executed:

#!/bin/bash
# Check the exception handling - does it catch narrowing exceptions?
# Also look for std::length_error or std::out_of_range in the codebase
rg -n 'std::length_error|std::out_of_range|numeric_limits' framework/vst/

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Verify the actual type signature of Steinberg::int32 and its limits
rg -n 'typedef.*int32|using.*int32' framework/ --type h | grep -i steinberg | head -10

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check if there's any platform-specific definition of Steinberg types
find framework -name "*.h" -type f -exec grep -l "Steinberg" {} \; | xargs grep -l "int32" | head -5 | xargs cat | grep -A3 -B3 "int32" | head -30

Repository: musescore/muse_framework

Length of output: 1051


🏁 Script executed:

#!/bin/bash
# Look for any VST SDK includes that might define these types
rg -n 'include' framework/vst/internal/vstplugininstance.cpp | head -15

Repository: musescore/muse_framework

Length of output: 237


🏁 Script executed:

#!/bin/bash
# Check what config sources these state strings come from
rg -B30 'componentState.*config.end' framework/vst/internal/vstplugininstance.cpp | grep -A30 'config' | head -40

Repository: musescore/muse_framework

Length of output: 1055


🏁 Script executed:

#!/bin/bash
# Look at the function signature of the applyConfig call to understand config origin
rg -n 'void.*applyConfig|applyConfig.*{' framework/vst/internal/vstplugininstance.cpp -A5 | head -20

Repository: musescore/muse_framework

Length of output: 196


🏁 Script executed:

#!/bin/bash
# Check if state blobs have any size limits documented or enforced earlier
rg -n 'size.*limit|limit.*size|MAX.*BYTE|BYTE.*MAX' framework/vst/ -i

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Let's verify the exact type definitions by checking the Steinberg SDK headers
# First, find where Steinberg types are defined/included
find framework -name "*.h" -type f | xargs grep -l "namespace Steinberg" | head -3

Repository: musescore/muse_framework

Length of output: 50


🏁 Script executed:

#!/bin/bash
# Check the audio::AudioUnitConfig type to understand config structure
rg -n 'using AudioUnitConfig|class AudioUnitConfig|struct AudioUnitConfig' framework/ --type h -A5

Repository: musescore/muse_framework

Length of output: 531


🏁 Script executed:

#!/bin/bash
# Verify the actual behavior: check if std::string can hold data larger than INT32_MAX in practice
# and whether there are any safeguards in config serialization
rg -n 'AudioUnitConfig|saveConfig|loadConfig' framework/vst/ | head -20

Repository: musescore/muse_framework

Length of output: 1650


Add bounds validation before narrowing state buffer size to Steinberg::int32.

Line 49 casts serialized state size from size_t to Steinberg::int32 without checking the upper bound. A std::string can exceed INT32_MAX on 64-bit systems; silent narrowing would truncate the state blob and corrupt plugin state restoration. Add a bounds check before casting.

Proposed fix
+#include <limits>
+#include <stdexcept>
+
 static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize)
 {
+    buffer.setSize(0);
+
     if (strSize == 0) {
         return;
     }
 
-    buffer.setSize(0);
+    if (strSize > static_cast<size_t>(std::numeric_limits<Steinberg::int32>::max())) {
+        throw std::length_error("VST state buffer is too large");
+    }
+
     buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
     buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize)
{
if (strSize == 0) {
return;
}
buffer.setSize(0);
buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
`#include` <limits>
`#include` <stdexcept>
static void stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize)
{
buffer.setSize(0);
if (strSize == 0) {
return;
}
if (strSize > static_cast<size_t>(std::numeric_limits<Steinberg::int32>::max())) {
throw std::length_error("VST state buffer is too large");
}
buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
}
🧰 Tools
🪛 Cppcheck (2.21.0)

[style] 45-45: The function 'modularity_isGlobalInterface' is never used.

(unusedFunction)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@framework/vst/internal/vstplugininstance.cpp` around lines 42 - 50, The
stateBufferFromString function casts the strSize parameter from size_t to
Steinberg::int32 without validating that the value fits within the int32 range,
which could cause silent truncation on 64-bit systems. Before the buffer.write
call, add a bounds check to ensure strSize does not exceed INT32_MAX, and either
return early or throw an error if the size is invalid.

}

VstPluginInstance::VstPluginInstance(const muse::audio::AudioResourceId& resourceId)
: m_resourceId(resourceId), m_componentHandlerPtr(new VstComponentHandler())
{
Expand All @@ -47,12 +58,18 @@ VstPluginInstance::VstPluginInstance(const muse::audio::AudioResourceId& resourc
m_id = ++s_lastId;

m_componentHandlerPtr->pluginParamsChanged().onNotify(this, [this]() {
rescanParams();
Async::call(this, [this]() {
rescanParams();
}, threadSecurer()->mainThreadId());
});
}

VstPluginInstance::~VstPluginInstance()
{
//! NOTE: Signal early so any rescanParams() already queued to the main thread
//! returns before touching members (provider, etc.)
m_isLoaded = false;

muse::audio::AudioResourceId resourceId = m_resourceId;
std::shared_ptr<VstPluginProvider> provider = std::move(m_pluginProvider);
PluginModulePtr module = std::move(m_module);
Expand Down Expand Up @@ -107,8 +124,6 @@ void VstPluginInstance::load()
Async::call(this, [this]() {
ONLY_MAIN_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

m_module = modulesRepo()->pluginModule(m_resourceId);
if (!m_module) {
modulesRepo()->addPluginModule(m_resourceId);
Expand All @@ -128,7 +143,6 @@ void VstPluginInstance::load()
}

m_pluginProvider = std::make_unique<VstPluginProvider>(factory, classInfo);
m_classInfo = classInfo;
break;
}

Expand Down Expand Up @@ -157,6 +171,8 @@ void VstPluginInstance::load()

void VstPluginInstance::syncControllerToComponentState()
{
ONLY_MAIN_THREAD(threadSecurer);

// Synchronize controller to the component's default state.
// Some plugins (e.g. Roland Cloud ZENOLOGY) rely on this to
// fully initialize internal data structures; without it the
Expand All @@ -183,9 +199,9 @@ void VstPluginInstance::syncControllerToComponentState()

void VstPluginInstance::rescanParams()
{
ONLY_AUDIO_OR_MAIN_THREAD(threadSecurer);
ONLY_MAIN_THREAD(threadSecurer);

if (!m_isLoaded || m_updatingState) {
if (!m_isLoaded) {
return;
}

Expand Down Expand Up @@ -232,97 +248,14 @@ void VstPluginInstance::rescanParams()
m_pluginSettingsChanges.send(updatedConfig);
}

void VstPluginInstance::stateBufferFromString(VstMemoryStream& buffer, char* strData, const size_t strSize) const
{
if (strSize == 0) {
return;
}

buffer.setSize(0);
buffer.write(strData, static_cast<Steinberg::int32>(strSize), nullptr);
buffer.seek(0, Steinberg::IBStream::kIBSeekSet, nullptr);
}

PluginViewPtr VstPluginInstance::createView() const
void VstPluginInstance::setPluginConfig(const audio::AudioUnitConfig& config)
{
ONLY_MAIN_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

if (!m_pluginProvider) {
return nullptr;
}

PluginControllerPtr controller = m_pluginProvider->controller();
if (!controller) {
return nullptr;
}

return owned(controller->createView(PluginEditorViewType::kEditor));
}

PluginControllerPtr VstPluginInstance::controller() const
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

if (!m_pluginProvider) {
return nullptr;
}

return m_pluginProvider->controller();
}

PluginComponentPtr VstPluginInstance::component() const
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

if (!m_pluginProvider) {
return nullptr;
}

return m_pluginProvider->component();
}

PluginMidiMappingPtr VstPluginInstance::midiMapping() const
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

if (!m_pluginProvider) {
return nullptr;
if (!m_isLoaded) {
return;
}

return m_pluginProvider->midiMapping();
}

bool VstPluginInstance::isAbleForInput() const
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

auto search = std::find_if(m_classInfo.subCategories().begin(),
m_classInfo.subCategories().end(), [](const std::string& subCategoryStr) {
return subCategoryStr == PluginSubCategory::Synth
|| subCategoryStr == PluginSubCategory::Piano
|| subCategoryStr == PluginSubCategory::Drum
|| subCategoryStr == PluginSubCategory::External;
});

return search != m_classInfo.subCategories().cend();
}

void VstPluginInstance::updatePluginConfig(const muse::audio::AudioUnitConfig& config)
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);

if (!m_pluginProvider) {
LOGE() << "Plugin provider is not initialized";
return;
Expand All @@ -343,9 +276,9 @@ void VstPluginInstance::updatePluginConfig(const muse::audio::AudioUnitConfig& c
return;
}

m_updatingState = true;
m_componentHandlerPtr->setSuppressNotify(true);
DEFER {
m_updatingState = false;
m_componentHandlerPtr->setSuppressNotify(false);
};

try {
Expand All @@ -372,27 +305,70 @@ void VstPluginInstance::updatePluginConfig(const muse::audio::AudioUnitConfig& c
}
}

void VstPluginInstance::refreshConfig()
PluginViewPtr VstPluginInstance::createView() const
{
ONLY_MAIN_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);
if (!m_isLoaded || !m_pluginProvider) {
return nullptr;
}

PluginControllerPtr controller = m_pluginProvider->controller();
if (!controller) {
return nullptr;
}

rescanParams();
return owned(controller->createView(PluginEditorViewType::kEditor));
}

bool VstPluginInstance::isValid() const
PluginControllerPtr VstPluginInstance::controller() const
{
ONLY_AUDIO_THREAD(threadSecurer);

std::lock_guard lock(m_mutex);
if (!m_isLoaded || !m_pluginProvider) {
return nullptr;
}

return m_pluginProvider->controller();
}

if (!m_module
|| !m_pluginProvider) {
return false;
PluginComponentPtr VstPluginInstance::component() const
{
// TODO: Audio engine or process thread
// ONLY_AUDIO_THREAD(threadSecurer);

if (!m_isLoaded || !m_pluginProvider) {
return nullptr;
}

return true;
return m_pluginProvider->component();
}

PluginMidiMappingPtr VstPluginInstance::midiMapping() const
{
ONLY_AUDIO_THREAD(threadSecurer);

if (!m_isLoaded || !m_pluginProvider) {
return nullptr;
}

return m_pluginProvider->midiMapping();
}

void VstPluginInstance::updatePluginConfig(const audio::AudioUnitConfig& config)
{
ONLY_AUDIO_THREAD(threadSecurer);

Async::call(this, [this, config]() {
setPluginConfig(config);
}, threadSecurer()->mainThreadId());
}

void VstPluginInstance::refreshConfig()
{
ONLY_MAIN_THREAD(threadSecurer);

rescanParams();
}

bool VstPluginInstance::isLoaded() const
Expand Down
Loading