From 19573df41a7e4fa10390f78cd3c35efc6c6e1537 Mon Sep 17 00:00:00 2001 From: hangzqcom Date: Sun, 25 Oct 2026 19:12:28 -0700 Subject: [PATCH 1/2] TACDev: bounds-check indexed getters and report GetHelpText size Several TACDev C API getters trusted the caller's index: - GetPortData() only checked deviceIndex against the upper bound, so a negative index was passed to QList::at() and crashed the host process (assert in debug, out-of-bounds read in release). - GetCommand() relied on commandEntry() returning pin 0 for a bad index, but it returns a default entry with pin -1, so an out-of-range index succeeded and produced a garbage ";;18446744073709551615;;;" entry instead of an error. - GetScriptVariable() did not check the index at all. Both now return TAC_BAD_INDEX with an empty buffer for an out-of-range index, the same as GetQuickCommand() already does. Existing callers iterate up to the reported count, so valid indexes behave exactly as before. GetHelpText() set *actualSize only when the buffer was too small. It now also reports the size on success, as the parameter name implies. Verified on a PSoC board: negative GetPortData indexes return 0 without crashing, out-of-range GetCommand/GetScriptVariable return TAC_BAD_INDEX, GetHelpText reports actualSize, and TacService, TACDump and the TAC GUI tools work unchanged. Signed-off-by: hangzqcom --- interfaces/C++/TACDev/TACDev.cpp | 34 ++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/interfaces/C++/TACDev/TACDev.cpp b/interfaces/C++/TACDev/TACDev.cpp index e32d26a6..8e93dc0f 100644 --- a/interfaces/C++/TACDev/TACDev.cpp +++ b/interfaces/C++/TACDev/TACDev.cpp @@ -247,7 +247,7 @@ unsigned long GetPortData int result{0}; const AlpacaDevices& alpacaDevices = gDevTACCore.GetAlpacaDevices(); - if (deviceIndex < alpacaDevices.count()) + if (deviceIndex >= 0 && deviceIndex < alpacaDevices.count()) { AlpacaDevice alpacaDevice = alpacaDevices.at(deviceIndex); QByteArray portData2; @@ -919,8 +919,13 @@ TAC_ERROR GetCommand AlpacaDevice alpacaDevice = gDevTACCore.getAlpacaDevice(tacHandle); if (alpacaDevice.isNull() == false) { - TACCommand commandEntry = alpacaDevice->commandEntry(commandIndex); - if (commandEntry._pin != 0) + // commandEntry() returns a default entry (pin == -1) for an out-of-range index, + // so check the range explicitly instead of relying on the pin value. + TACCommand commandEntry; + const bool inRange = commandIndex < alpacaDevice->commandCount(); + if (inRange) + commandEntry = alpacaDevice->commandEntry(commandIndex); + if (inRange && commandEntry._pin != 0) { QByteArray commandData; @@ -1102,7 +1107,11 @@ TAC_ERROR GetHelpText *actualSize = helpText.size(); } else + { my_memcpy(helpBuffer, bufferSize, helpText, helpText.size()); + if (actualSize != nullptr) + *actualSize = helpText.size(); + } } else { @@ -1140,14 +1149,23 @@ TAC_ERROR GetScriptVariable(TAC_HANDLE tacHandle, unsigned long scriptVariableIn if (alpacaDevice.isNull() == false) { - QByteArray variableData = alpacaDevice->getScriptVariable(scriptVariableIndex); - if (variableData.size() >= bufferSize) + if (scriptVariableIndex >= alpacaDevice->scriptVariableCount()) { - result = TACDEV_BUFFER_TOO_SMALL; - gDevTACCore.setLastError(kTACDevBufferTooSmall); + my_memcpy(scriptVariableBuffer, bufferSize, "\0", 1); + result = TAC_BAD_INDEX; + gDevTACCore.setLastError(kTACBadIndex); } else - my_memcpy(scriptVariableBuffer, bufferSize, variableData, variableData.size()); + { + QByteArray variableData = alpacaDevice->getScriptVariable(scriptVariableIndex); + if (variableData.size() >= bufferSize) + { + result = TACDEV_BUFFER_TOO_SMALL; + gDevTACCore.setLastError(kTACDevBufferTooSmall); + } + else + my_memcpy(scriptVariableBuffer, bufferSize, variableData, variableData.size()); + } } else { From 585b3062180a1dd6c885e5224fab1c4ec3ccda2a Mon Sep 17 00:00:00 2001 From: hangzqcom Date: Mon, 26 Oct 2026 15:20:46 -0700 Subject: [PATCH 2/2] TACDev: guard every GetHelpText actualSize write GetHelpText wrote *actualSize without a null check on the buffer-too-small and bad-handle paths. Only the success path was guarded. Guard all three so a caller may pass nullptr for actualSize. Also drop a stale todo comment; the function is fully implemented. Signed-off-by: hangzqcom --- interfaces/C++/TACDev/TACDev.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/interfaces/C++/TACDev/TACDev.cpp b/interfaces/C++/TACDev/TACDev.cpp index 8e93dc0f..666af33b 100644 --- a/interfaces/C++/TACDev/TACDev.cpp +++ b/interfaces/C++/TACDev/TACDev.cpp @@ -1098,13 +1098,13 @@ TAC_ERROR GetHelpText AlpacaDevice alpacaDevice = gDevTACCore.getAlpacaDevice(tacHandle); if (alpacaDevice.isNull() == false) { - // todo QByteArray helpText = alpacaDevice->getHelp(); if (helpText.size() >= bufferSize) { result = TACDEV_BUFFER_TOO_SMALL; gDevTACCore.setLastError(kTACDevBufferTooSmall); - *actualSize = helpText.size(); + if (actualSize != nullptr) + *actualSize = helpText.size(); } else { @@ -1116,7 +1116,8 @@ TAC_ERROR GetHelpText else { my_memcpy(helpBuffer, bufferSize, "\0", 1); - *actualSize = 0; + if (actualSize != nullptr) + *actualSize = 0; result = TACDEV_BAD_TAC_HANDLE; gDevTACCore.setLastError(kTACDevHandleNotOpen);