From 0fcd67f91156006fdacb94a5c5c9bcd767600f08 Mon Sep 17 00:00:00 2001 From: Fawn Date: Thu, 23 Oct 2025 12:58:32 -0700 Subject: [PATCH] Use length-bounded string/memory functions (#7709) Resolves #3949 by replacing all calls to sprintf() and strcpy() in first-party code with calls to snprintf() or some other reasonable alternative. --- include/DrumSynth.h | 8 +- plugins/ReverbSC/base.c | 44 +++-------- plugins/VstBase/RemoteVstPlugin.cpp | 115 +++++++++++++++------------- src/core/DrumSynth.cpp | 90 +++++++++------------- src/core/main.cpp | 6 +- 5 files changed, 115 insertions(+), 148 deletions(-) diff --git a/include/DrumSynth.h b/include/DrumSynth.h index 2894817fe..3281dddda 100644 --- a/include/DrumSynth.h +++ b/include/DrumSynth.h @@ -26,7 +26,8 @@ #ifndef LMMS_DRUM_SYNTH_H #define LMMS_DRUM_SYNTH_H -#include +#include +#include #include "LmmsTypes.h" @@ -48,8 +49,9 @@ private: float waveform(float ph, int form); - int GetPrivateProfileString( - const char* sec, const char* key, const char* def, char* buffer, int size, QString file); + std::size_t GetPrivateProfileString(const char* sec, + const char* key, const char* def, char* buffer, + std::size_t size, QString file); int GetPrivateProfileInt(const char* sec, const char* key, int def, QString file); float GetPrivateProfileFloat(const char* sec, const char* key, float def, QString file); }; diff --git a/plugins/ReverbSC/base.c b/plugins/ReverbSC/base.c index d5c52b0bd..49555cee9 100644 --- a/plugins/ReverbSC/base.c +++ b/plugins/ReverbSC/base.c @@ -4,44 +4,24 @@ #include #include "base.h" -int sp_create(sp_data **spp) -{ - *spp = (sp_data *) malloc(sizeof(sp_data)); - sp_data *sp = *spp; - sprintf(sp->filename, "test.wav"); - sp->nchan = 1; - SPFLOAT *out = malloc(sizeof(SPFLOAT) * sp->nchan); - *out = 0; - sp->out = out; - sp->sr = 44100; - sp->len = 5 * sp->sr; - sp->pos = 0; - sp->rand = 0; - return 0; -} +int sp_create(sp_data **spp) { return sp_createn(spp, 1); } int sp_createn(sp_data **spp, int nchan) { - *spp = (sp_data *) malloc(sizeof(sp_data)); - sp_data *sp = *spp; - sprintf(sp->filename, "test.wav"); - sp->nchan = nchan; - SPFLOAT *out = malloc(sizeof(SPFLOAT) * sp->nchan); - *out = 0; - sp->out = out; - sp->sr = 44100; - sp->len = 5 * sp->sr; - sp->pos = 0; - sp->rand = 0; - return 0; + const uint32_t sr = 44100; // TODO C23: constexpr auto + const unsigned long len_seconds = 5; // TODO C23: constexpr auto + *spp = malloc(sizeof(sp_data)); + **spp = (sp_data){ .out = calloc(nchan, sizeof(float)), .sr = sr, + .nchan = nchan, .len = len_seconds * sr, .pos = 0, + .filename = "test.wav", .rand = 0 }; + return 0; } int sp_destroy(sp_data **spp) { - sp_data *sp = *spp; - free(sp->out); - free(*spp); - return 0; + free((*spp)->out); + free(*spp); + return 0; } #ifndef NO_LIBSNDFILE @@ -61,7 +41,7 @@ int sp_process(sp_data *sp, void *ud, void (*callback)(sp_data *, void *)) sf[0] = sf_open(sp->filename, SFM_WRITE, &info); } else { for(chan = 0; chan < sp->nchan; chan++) { - sprintf(tmp, "%02d_%s", chan, sp->filename); + snprintf(tmp, sizeof(tmp), "%02d_%s", chan, sp->filename); sf[chan] = sf_open(tmp, SFM_WRITE, &info); } } diff --git a/plugins/VstBase/RemoteVstPlugin.cpp b/plugins/VstBase/RemoteVstPlugin.cpp index 0a7680094..748ed1046 100644 --- a/plugins/VstBase/RemoteVstPlugin.cpp +++ b/plugins/VstBase/RemoteVstPlugin.cpp @@ -30,6 +30,7 @@ * */ + #include #include "lmmsconfig.h" @@ -1010,13 +1011,14 @@ bool RemoteVstPlugin::load( const std::string & _plugin_file ) return false; } + const char id[5] = { + static_cast(m_plugin->uniqueID >> 24), + static_cast(m_plugin->uniqueID >> 16), + static_cast(m_plugin->uniqueID >> 8), + static_cast(m_plugin->uniqueID ), + 0 + }; - char id[5]; - sprintf( id, "%c%c%c%c", ((char *)&m_plugin->uniqueID)[3], - ((char *)&m_plugin->uniqueID)[2], - ((char *)&m_plugin->uniqueID)[1], - ((char *)&m_plugin->uniqueID)[0] ); - id[4] = 0; sendMessage( message( IdVstPluginUniqueID ).addString( id ) ); pluginDispatch( effOpen ); @@ -1244,10 +1246,14 @@ void RemoteVstPlugin::getParameterLabels() void RemoteVstPlugin::sendCurrentProgramName() { - char presName[64]; - sprintf( presName, "%d/%d: %s", pluginDispatch( effGetProgram ) + 1, m_plugin->numPrograms, programName() ); - - sendMessage( message( IdVstCurrentProgramName ).addString( presName ) ); + char presName[64] = {}; + std::snprintf(presName, sizeof(presName), + "%d/%d: %s", + pluginDispatch(effGetProgram) + 1, + m_plugin->numPrograms, + programName() + ); + sendMessage(message(IdVstCurrentProgramName).addString(presName)); } @@ -1372,36 +1378,46 @@ void RemoteVstPlugin::rotateProgram( int offset ) void RemoteVstPlugin::getProgramNames() { - char presName[1024+256*30]; - char curProgName[30]; - if (isInitialized() == false) return; - bool progNameIndexed = pluginDispatch(effGetProgramNameIndexed, 0, -1, curProgName) == 1; + char presName[1024 + 256 * 30] = {}; + char curProgName[30] = {}; + if (!isInitialized()) { return; } + const bool progNameIndexed = pluginDispatch(effGetProgramNameIndexed, 0, -1, curProgName) == 1; - if (m_plugin->numPrograms > 1) { - if (progNameIndexed) { - for (int i = 0; i< (m_plugin->numPrograms >= 256?256:m_plugin->numPrograms); i++) + if (m_plugin->numPrograms > 1) + { + const auto maxPrograms = std::min(m_plugin->numPrograms, 256); + if (progNameIndexed) + { + for (int i = 0; i < maxPrograms; i++) { pluginDispatch(effGetProgramNameIndexed, i, -1, curProgName); - if (i == 0) sprintf( presName, "%s", curProgName ); - else sprintf( presName + strlen(presName), "|%s", curProgName ); + if (i == 0) { std::snprintf(presName, sizeof(presName), "%s", curProgName); } + else + { + const auto len = std::strlen(presName); + std::snprintf(presName + len, sizeof(presName) - len, "|%s", curProgName); + } } } else { - int currProgram = pluginDispatch( effGetProgram ); - for (int i = 0; i< (m_plugin->numPrograms >= 256?256:m_plugin->numPrograms); i++) + const int currProgram = pluginDispatch(effGetProgram); + for (int i = 0; i < maxPrograms; i++) { - pluginDispatch( effSetProgram, 0, i ); - if (i == 0) sprintf( presName, "%s", programName() ); - else sprintf( presName + strlen(presName), "|%s", programName() ); + pluginDispatch(effSetProgram, 0, i); + if (i == 0) { std::snprintf(presName, sizeof(presName), "%s", programName()); } + else + { + const auto len = std::strlen(presName); + std::snprintf(presName + len, sizeof(presName) - len, "|%s", programName()); + } } - pluginDispatch( effSetProgram, 0, currProgram ); + pluginDispatch(effSetProgram, 0, currProgram); } - } else sprintf( presName, "%s", programName() ); + } + else { std::snprintf(presName, sizeof(presName), "%s", programName()); } - presName[sizeof(presName)-1] = 0; - - sendMessage( message( IdVstProgramNames ).addString( presName ) ); + sendMessage(message(IdVstProgramNames).addString(presName)); } @@ -1725,19 +1741,12 @@ int RemoteVstPlugin::updateInOutCount() setInputOutputCount( inputCount(), outputCount() ); - char buf[64]; - sprintf( buf, "inputs: %d output: %d\n", inputCount(), outputCount() ); - debugMessage( buf ); + char buf[64] = {}; + std::snprintf(buf, sizeof(buf), "inputs: %d; outputs: %d\n", inputCount(), outputCount()); + debugMessage(buf); - if( inputCount() > 0 ) - { - m_inputs = new float * [inputCount()]; - } - - if( outputCount() > 0 ) - { - m_outputs = new float * [outputCount()]; - } + if (inputCount() > 0) { m_inputs = new float*[inputCount()]; } + if (outputCount() > 0) { m_outputs = new float*[outputCount()]; } return 1; } @@ -1765,9 +1774,9 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, { static VstTimeInfo _timeInfo; #ifdef DEBUG_CALLBACKS - char buf[64]; - sprintf( buf, "host-callback, opcode = %d\n", (int) _opcode ); - SHOW_CALLBACK( buf ); + char buf[64] = {}; + std::snprintf(buf, sizeof(buf), "host-callback, opcode = %d\n", static_cast(_opcode)); + SHOW_CALLBACK(buf); #endif // workaround for early callbacks by some plugins @@ -1776,6 +1785,7 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, __plugin->m_plugin = _effect; } + const auto p = static_cast(_ptr); switch( _opcode ) { case audioMasterAutomate: @@ -2070,15 +2080,14 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, SHOW_CALLBACK( "amc: audioMasterGetVendorString\n" ); // fills with a string identifying the vendor // (max 64 char) - strcpy( (char *) _ptr, "Tobias Doerffel" ); + std::strcpy(p, "Tobias Doerffel"); return 1; case audioMasterGetProductString: SHOW_CALLBACK( "amc: audioMasterGetProductString\n" ); // fills with a string with product name // (max 64 char) - strcpy( (char *) _ptr, - "LMMS VST Support Layer (LVSL)" ); + std::strcpy(p, "LMMS VST Support Layer (LVSL)"); return 1; case audioMasterGetVendorVersion: @@ -2092,12 +2101,12 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, return 0; case audioMasterCanDo: - SHOW_CALLBACK( "amc: audioMasterCanDo\n" ); - return !strcmp( (char *) _ptr, "sendVstEvents" ) || - !strcmp( (char *) _ptr, "sendVstMidiEvent" ) || - !strcmp( (char *) _ptr, "sendVstTimeInfo" ) || - !strcmp( (char *) _ptr, "sizeWindow" ) || - !strcmp( (char *) _ptr, "supplyIdle" ); + SHOW_CALLBACK("amc: audioMasterCanDo\n"); + return !(std::strcmp(p, "sendVstEvents") + && std::strcmp(p, "sendVstMidiEvent") + && std::strcmp(p, "sendVstTimeInfo") + && std::strcmp(p, "sizeWindow") + && std::strcmp(p, "supplyIdle")); case audioMasterGetLanguage: SHOW_CALLBACK( "amc: audioMasterGetLanguage\n" ); diff --git a/src/core/DrumSynth.cpp b/src/core/DrumSynth.cpp index 5cf836ba0..6705d78b2 100644 --- a/src/core/DrumSynth.cpp +++ b/src/core/DrumSynth.cpp @@ -26,6 +26,7 @@ #include "DrumSynth.h" #include +#include #include #include #include @@ -185,14 +186,14 @@ float DrumSynth::waveform(float ph, int form) return (saw01 < 0.5f) ? 1.f : -1.f; } -int DrumSynth::GetPrivateProfileString( - const char* sec, const char* key, const char* def, char* buffer, int size, QString file) +std::size_t DrumSynth::GetPrivateProfileString(const char* sec, + const char* key, const char* def, char* buffer, std::size_t size, + QString file) { + const auto maxlen = std::max(std::size_t{1}, size) - 1; // TODO C++26: Use std::sub_sat(size, 1) stringstream is; bool inSection = false; - int len = 0; - - char* line = static_cast(malloc(200)); + std::array line = {}; // Use QFile to handle unicode file name on Windows // Previously we used ifstream directly @@ -201,68 +202,47 @@ int DrumSynth::GetPrivateProfileString( QByteArray dat = f.readAll().constData(); is.str(string(dat.constData(), dat.size())); + // If buffer[0] isn't overwritten after reading file, default value + // `def` will be used. + std::memset(buffer, '\0', size); + while (is.good()) { if (!inSection) { is.ignore(numeric_limits::max(), '['); - if (!is.eof()) { - is.getline(line, 200, ']'); - if (strcasecmp(line, sec) == 0) - { - inSection = true; - } + is.getline(line.data(), line.size(), ']'); + if (!strcasecmp(line.data(), sec)) { inSection = true; } } } else if (!is.eof()) { - is.getline(line, 200); - if (line[0] == '[') + is.getline(line.data(), line.size()); + if (line[0] == '[') { break; } + + char* k = std::strtok(line.data(), " \t="); + char* b = std::strtok(nullptr, "\n\r\0"); + if (k && !strcasecmp(k, key)) { - break; - } - - char* k = strtok(line, " \t="); - char* b = strtok(nullptr, "\n\r\0"); - - if (k != 0 && strcasecmp(k, key) == 0) - { - if (b == 0) + if (b) { - len = 0; - buffer[0] = 0; - } - else - { - k = static_cast(b + strlen(b) - 1); - while ((k >= b) && (*k == ' ' || *k == '\t')) - { - --k; - } - *(k + 1) = '\0'; - - len = strlen(b); - if (len > size - 1) - { - len = size - 1; - } - strncpy(buffer, b, len + 1); + // Trim trailing whitespace + k = &b[std::strlen(b)]; + while (k >= b && (*k == ' ' || *k == '\t')) { --k; } + k[1] = '\0'; // k == b - 1 when string is empty or all whitespace + std::strncpy(buffer, b, maxlen); } break; } } } - if (len == 0) - { - len = strlen(def); - strncpy(buffer, def, size); - } - - free(line); - + // Use default value `def` if value is missing + if (!buffer[0]) { std::strncpy(buffer, def, maxlen); } + // Since nothing ever copies past `maxlen`, buffer must be null-terminated + const auto len = std::strlen(buffer); return len; } @@ -345,7 +325,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa } // try to read version from input file - strcpy(sec, "General"); + std::strcpy(sec, "General"); GetPrivateProfileString(sec, "Version", "", ver, sizeof(ver), dsfile); ver[9] = 0; if ((strcasecmp(ver, "DrumSynth") != 0) // input fail @@ -398,7 +378,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa GetEnv(7, sec, "FilterEnv", dsfile); // read noise parameters - strcpy(sec, "Noise"); + std::strcpy(sec, "Noise"); chkOn[1] = GetPrivateProfileInt(sec, "On", 0, dsfile); sliLev[1] = GetPrivateProfileInt(sec, "Level", 0, dsfile); NT = GetPrivateProfileInt(sec, "Slope", 0, dsfile); @@ -423,7 +403,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa // srand(1); //fixed random sequence // read tone parameters - strcpy(sec, "Tone"); + std::strcpy(sec, "Tone"); chkOn[0] = GetPrivateProfileInt(sec, "On", 0, dsfile); TON = chkOn[0]; sliLev[0] = GetPrivateProfileInt(sec, "Level", 128, dsfile); @@ -452,7 +432,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa Tphi = GetPrivateProfileFloat(sec, "Phase", 90.f, dsfile) / 57.29578f; // degrees>radians // read overtone parameters - strcpy(sec, "Overtones"); + std::strcpy(sec, "Overtones"); chkOn[2] = GetPrivateProfileInt(sec, "On", 0, dsfile); OON = chkOn[2]; sliLev[2] = GetPrivateProfileInt(sec, "Level", 128, dsfile); @@ -497,7 +477,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa } // read noise band parameters - strcpy(sec, "NoiseBand"); + std::strcpy(sec, "NoiseBand"); chkOn[3] = GetPrivateProfileInt(sec, "On", 0, dsfile); BON = chkOn[3]; sliLev[3] = GetPrivateProfileInt(sec, "Level", 128, dsfile); @@ -510,7 +490,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa BQ = BQ * BQ / (10000.f - 6600.f * (std::sqrt(BF) - 0.19f)); BFStep = 1 + static_cast((40.f - (BFStep / 2.5f)) / (BQ + 1.f + (1.f * BF))); - strcpy(sec, "NoiseBand2"); + std::strcpy(sec, "NoiseBand2"); chkOn[4] = GetPrivateProfileInt(sec, "On", 0, dsfile); BON2 = chkOn[4]; sliLev[4] = GetPrivateProfileInt(sec, "Level", 128, dsfile); @@ -524,7 +504,7 @@ int DrumSynth::GetDSFileSamples(QString dsfile, int16_t*& wave, int channels, sa BFStep2 = 1 + static_cast((40 - (BFStep2 / 2.5)) / (BQ2 + 1 + (1 * BF2))); // read distortion parameters - strcpy(sec, "Distortion"); + std::strcpy(sec, "Distortion"); chkOn[5] = GetPrivateProfileInt(sec, "On", 0, dsfile); DiON = chkOn[5]; DStep = 1 + GetPrivateProfileInt(sec, "Rate", 0, dsfile); diff --git a/src/core/main.cpp b/src/core/main.cpp index a6800f4e8..3f83339b0 100644 --- a/src/core/main.cpp +++ b/src/core/main.cpp @@ -286,11 +286,7 @@ int main( int argc, char * * argv ) } else if (arg == "--geometry" || arg == "-geometry") { - if (arg == "--geometry") - { - // Delete the first "-" so Qt recognize the option - strcpy(argv[i], "-geometry"); - } + if (arg == "--geometry") { argv[i]++; } // Delete the first "-" so Qt recognize the option // option -geometry is filtered by Qt later, // so we need to check its presence now to // determine, if the application should run in