SarAsio: don't crash the host on long endpoint names or bad channel counts

SarAsio/configui.cpp

@@ -18,6 +18,7 @@
 #include "configui.h"
 #include "dllmain.h"
 #include "utility.h"
+#include "sar.h"
 
 using namespace Sar;
 
@@ -390,6 +391,8 @@ void EndpointsPropertySheetPage::initEpDialogControls()
     _epDialogType = GetDlgItem(_epDialog, 1103);
     _epDialogChannelCount = GetDlgItem(_epDialog, 1105);
 
+    Edit_LimitText(_epDialogName, MAX_ENDPOINT_NAME_LENGTH);
+
     ComboBox_AddString(_epDialogType, L"Playback");
     ComboBox_AddString(_epDialogType, L"Recording");
     ComboBox_SetCurSel(_epDialogType,
@@ -402,7 +405,7 @@ void EndpointsPropertySheetPage::initEpDialogControls()
     Edit_SetText(_epDialogChannelCount, wos.str().c_str());
 }
 
-void EndpointsPropertySheetPage::updateEpDialogConfig()
+bool EndpointsPropertySheetPage::updateEpDialogConfig()
 {
     auto len = Edit_GetTextLength(_epDialogName);
     auto buf = new WCHAR[len + 1];
@@ -415,12 +418,28 @@ void EndpointsPropertySheetPage::updateEpDialogConfig()
     buf = new WCHAR[len + 1];
     Edit_GetText(_epDialogChannelCount, buf, len + 1);
 
-    auto countStr = TCHARToUTF8(buf);
+    WCHAR *end = nullptr;
+    long count = wcstol(buf, &end, 10);
+    bool valid = end != buf && *end == L'\0' &&
+        count >= 1 && count <= SAR_MAX_CHANNEL_COUNT;
 
     delete[] buf;
-    _epDialogConfig.channelCount = max(1, std::stoi(countStr));
+
+    if (!valid) {
+        // std::stoi threw on these and took the host down with it.
+        std::wostringstream message;
+
+        message << L"The channel count must be a number from 1 to "
+            << SAR_MAX_CHANNEL_COUNT << L".";
+        MessageBox(_epDialog, message.str().c_str(), TEXT("Error"),
+            MB_OK | MB_ICONERROR);
+        return false;
+    }
+
+    _epDialogConfig.channelCount = (int)count;
     _epDialogConfig.type = ComboBox_GetCurSel(_epDialogType) == 1 ?
         EndpointType::Recording : EndpointType::Playback;
+    return true;
 }
 
 INT_PTR EndpointsPropertySheetPage::epDialogProc(
@@ -435,7 +454,9 @@ INT_PTR EndpointsPropertySheetPage::epDialogProc(
         case WM_COMMAND:
             switch (LOWORD(wparam)) {
                 case IDOK:
-                    updateEpDialogConfig();
+                    if (!updateEpDialogConfig()) {
+                        break;
+                    }
 
                     // fall-through
                 case IDCANCEL:

SarAsio/configui.h

@@ -100,7 +100,7 @@ struct EndpointsPropertySheetPage: public PropertySheetPage
     HWND _removeButton;
 
     void initEpDialogControls();
-    void updateEpDialogConfig();
+    bool updateEpDialogConfig();
     INT_PTR epDialogProc(
         HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam);
     static INT_PTR CALLBACK epDialogProcStub(

SarAsio/sarclient.cpp

@@ -397,8 +397,20 @@ bool SarClient::createEndpoints()
             SAR_ENDPOINT_TYPE_PLAYBACK : SAR_ENDPOINT_TYPE_RECORDING;
         request.channelCount = endpoint.channelCount;
         request.index = i++;
-        wcscpy_s(request.name, endpoint.description.c_str());
-        wcscpy_s(request.id, UTF8ToWide(endpoint.id).c_str());
+
+        // wcscpy_s would abort the host process on a value that doesn't fit.
+        // A long name is only cut short; a long ID can't be, since two IDs
+        // could then collide.
+        auto id = UTF8ToWide(endpoint.id);
+
+        if (id.size() > MAX_ENDPOINT_NAME_LENGTH) {
+            LOG(ERROR) << "Endpoint ID " << endpoint.id << " is longer than "
+                << MAX_ENDPOINT_NAME_LENGTH << " characters.";
+            return false;
+        }
+
+        wcsncpy_s(request.name, endpoint.description.c_str(), _TRUNCATE);
+        wcscpy_s(request.id, id.c_str());
 
         if (!DeviceIoControl(_device, SAR_CREATE_ENDPOINT,
             (LPVOID)&request, sizeof(request), nullptr, 0, &dummy, nullptr)) {

SarTest/SarTest.cpp

@@ -65,6 +65,7 @@ int usage()
         "\n"
         "Layout options: --endpoints N (playback/recording pairs, default 2)\n"
         "                --channels C (per endpoint, default 2) --prefix <name>\n"
+        "                --id-prefix <id> (endpoint IDs; default: the name prefix)\n"
         "Other options:  --results <file.json> --phase-timeout S --no-sarasio-log\n"
         "                --keep-config (host/run: do not rewrite default.json)\n"
         "                --registered (use the COM-registered SarAsio instead of a path)\n"

SarTest/common.cpp

@@ -216,6 +216,7 @@ EndpointLayout EndpointLayout::fromArgs(const Args& args)
     layout.pairs = args.getInt(L"endpoints", 2);
     layout.channels = args.getInt(L"channels", 2);
     layout.prefix = args.get(L"prefix", L"SarTest");
+    layout.idPrefix = args.get(L"id-prefix", L"");
 
     if (layout.pairs < 1) {
         layout.pairs = 1;

tools/test/Invoke-SarTest.ps1

@@ -153,6 +153,11 @@ if ($Scenarios -contains 'issues') {
         @('no-interface') + $common) -Timeout 120
     $summary.scenarios += Invoke-Scenario 'issue-15-endpoint-names' (
         @('endpoint-names') + $common) -Timeout 300
+    # An endpoint name longer than the driver's 63 characters used to abort
+    # the host process in SarAsio.
+    $summary.scenarios += Invoke-Scenario 'long-endpoint-name' (
+        @('host', '--endpoints', 1, '--duration', 2, '--id-prefix', 'SarTest-long',
+          '--prefix', ('SarTestLongEndpointName' * 4)) + $common) -Timeout 120
 }
 
 if ($Scenarios -contains 'matrix') {

tools/test/README.md

@@ -90,7 +90,9 @@ clock never had a second instance alive. It needs an interactive desktop.
 configured and checks that it still offers a stereo input and output.
 `SarTest.exe endpoint-names` (#15, #66) renames an endpoint keeping its ID,
 then gives it a new ID, and checks that the new ID shows the new name; the
-results record what Windows shows for the reused ID.
+results record what Windows shows for the reused ID. `long-endpoint-name`
+runs a host whose endpoint names are longer than the driver's 63-character
+limit.
 `Invoke-SarTest.ps1 -Scenarios` selects any of `issues`, `matrix`, `race`
 and `kill`.