From 14d1bdcab9698678d8587b36fe4265394915aa08 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 16 Jun 2026 10:38:42 +0900 Subject: [PATCH] Bound OSC index before reads in wolfSSH_DoOSC --- .github/workflows/windows-check.yml | 131 ++++++++++++++++++++++++++++ src/wolfterm.c | 19 +++- tests/api.c | 27 ++++++ 3 files changed, 175 insertions(+), 2 deletions(-) diff --git a/.github/workflows/windows-check.yml b/.github/workflows/windows-check.yml index cf4e955f2..89b48d3a6 100644 --- a/.github/workflows/windows-check.yml +++ b/.github/workflows/windows-check.yml @@ -65,3 +65,134 @@ jobs: # See https://docs.microsoft.com/visualstudio/msbuild/msbuild-command-line-reference run: msbuild /m /p:PlatformToolset=v142 /p:Platform=${{env.BUILD_PLATFORM}} /p:WindowsTargetPlatformVersion=${{env.TARGET_PLATFORM}} /p:Configuration=${{env.WOLFSSH_BUILD_CONFIGURATION}} ${{env.SOLUTION_FILE_PATH}} + # Build and run the self-contained unit tests with the MSVC AddressSanitizer. + # This is the only job that executes wolfSSH tests under a sanitizer on + # Windows, where the USE_WINDOWS_API console code (e.g. wolfSSH_DoOSC) is + # compiled, so it guards that path against out-of-bounds reads. + asan-tests: + runs-on: windows-latest + + env: + # print_stats=1 emits allocator stats at exit, positive evidence that the + # ASan runtime was active in-process during each test run. + ASAN_OPTIONS: abort_on_error=1:print_stats=1 + + steps: + - uses: actions/checkout@v2 + with: + repository: wolfssl/wolfssl + path: wolfssl + + - uses: actions/checkout@master + with: + path: wolfssh + + - name: Add MSBuild to PATH + uses: microsoft/setup-msbuild@v1 + + # The pinned v142 toolset does not ship the AddressSanitizer runtime libs on + # the runner. Find an installed MSVC toolset that does and build everything in + # this job with it, recording its lib/bin dirs for the link and run steps. + # wolfssl, wolfssh and the test projects must share one compiler version + # because /GL (whole program optimization) does code generation at link time. + - name: Locate MSVC AddressSanitizer runtime + shell: pwsh + run: | + $vs = & "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath + $lib = Get-ChildItem "$vs\VC\Tools\MSVC\*\lib\x64\clang_rt.asan_dynamic_runtime_thunk-x86_64.lib" -ErrorAction SilentlyContinue | + Sort-Object { [version]($_.FullName -replace '.*\\MSVC\\([0-9.]+)\\.*','$1') } | + Select-Object -Last 1 + if ($null -eq $lib) { throw "MSVC AddressSanitizer runtime not found in any installed toolset" } + $msvcDir = $lib.Directory.Parent.Parent.FullName + $ver = [version]($msvcDir.Split('\')[-1]) + if ($ver.Minor -ge 30) { $toolset = "v143" } else { $toolset = "v142" } + "ASAN_TOOLSET=$toolset" | Out-File $env:GITHUB_ENV -Append + "ASAN_LIB_DIR=$($lib.Directory.FullName)" | Out-File $env:GITHUB_ENV -Append + "ASAN_BIN_DIR=$(Join-Path $msvcDir 'bin\Hostx64\x64')" | Out-File $env:GITHUB_ENV -Append + Write-Host "Using toolset $toolset (MSVC $ver)" + + - name: Restore wolfSSL NuGet packages + working-directory: wolfssl + run: nuget restore ${{env.WOLFSSL_SOLUTION_FILE_PATH}} + + # These env paths already include the wolfssh/ and wolfssl/ checkout + # prefixes, so they must run from the workspace root (as the build job does). + - name: updated user_settings.h for sshd and x509 + run: cp ${{env.USER_SETTINGS_H_NEW}} ${{env.USER_SETTINGS_H}} + + - name: replace wolfSSL user_settings.h with wolfSSH user_settings.h + run: get-content ${{env.USER_SETTINGS_H_NEW}} | %{$_ -replace "if 0","if 1"} + + # WholeProgramOptimization=false disables /GL so the v142-independent objects + # link without a cross-version code-generation requirement (C1047). + - name: Build wolfssl library + working-directory: wolfssl + shell: pwsh + run: | + msbuild /m /p:PlatformToolset=$env:ASAN_TOOLSET /p:Platform=${{env.BUILD_PLATFORM}} /p:Configuration=${{env.WOLFSSL_BUILD_CONFIGURATION}} /p:WholeProgramOptimization=false /t:wolfssl ${{env.WOLFSSL_SOLUTION_FILE_PATH}} + if ($LASTEXITCODE -ne 0) { throw "wolfssl build failed" } + + - name: Restore NuGet packages + working-directory: wolfssh\ide\winvs + run: nuget restore ${{env.SOLUTION_FILE_PATH}} + + # EnableASAN=true adds /fsanitize=address to each test project and to the + # referenced wolfssh library project, instrumenting src/wolfterm.c. The + # AddressSanitizer lib dir is added to LIB so the runtime thunk resolves. + - name: Build api-test and unit-test with AddressSanitizer + working-directory: wolfssh\ide\winvs + shell: pwsh + run: | + $env:LIB = "$env:ASAN_LIB_DIR;$env:LIB" + foreach ($p in @("api-test\api-test.vcxproj", "unit-test\unit-test.vcxproj")) { + msbuild /m /p:PlatformToolset=$env:ASAN_TOOLSET /p:Platform=${{env.BUILD_PLATFORM}} /p:WindowsTargetPlatformVersion=${{env.TARGET_PLATFORM}} /p:Configuration=${{env.WOLFSSH_BUILD_CONFIGURATION}} /p:EnableASAN=true /p:WholeProgramOptimization=false $p + if ($LASTEXITCODE -ne 0) { throw "build failed: $p" } + } + + # Positive control: compile a deliberate heap overflow with the same toolset + # and confirm ASan aborts on it. This proves ASan detection actually works on + # this runner, so a clean test run is meaningful rather than a silent no-op. + - name: Verify AddressSanitizer detection (positive control) + shell: pwsh + run: | + $vs = & "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath + Import-Module "$vs\Common7\Tools\Microsoft.VisualStudio.DevShell.dll" + Enter-VsDevShell -VsInstallPath $vs -SkipAutomaticLocation -DevCmdArguments "-arch=x64" | Out-Null + Set-Content canary.c 'int main(int argc, char** argv) { volatile char b[1]; (void)argv; return b[argc + 10]; }' + cl /nologo /fsanitize=address /MD canary.c + if ($LASTEXITCODE -ne 0) { throw "canary failed to compile" } + $out = & .\canary.exe 2>&1 | Out-String + $code = $LASTEXITCODE + Write-Host $out + if ($code -eq 0 -or $out -notmatch "AddressSanitizer") { + throw "Positive control FAILED: ASan did not detect the deliberate overflow" + } + Write-Host "Positive control OK: ASan detected the deliberate out-of-bounds access" + # canary.exe aborts with a non-zero code by design; reset so the step + # itself reports success. + exit 0 + + # Building a .vcxproj directly leaves $(SolutionDir) unset, so each project + # writes to its own \Release\x64 dir. Run from the wolfssh repo root + # so the tests find keys/ and certs/, and copy the dynamic ASAN runtime next + # to each binary so it loads without a full developer-prompt environment. + - name: Run tests under AddressSanitizer + working-directory: wolfssh + shell: pwsh + run: | + $cfg = "${{env.WOLFSSH_BUILD_CONFIGURATION}}\${{env.BUILD_PLATFORM}}" + $dll = Join-Path $env:ASAN_BIN_DIR "clang_rt.asan_dynamic-x86_64.dll" + if (-not (Test-Path $dll)) { throw "ASAN runtime DLL not found: $dll" } + foreach ($t in @("api-test", "unit-test")) { + $dir = "ide\winvs\$t\$cfg" + # Static signal: confirm the binary really imports the ASan runtime, so + # we are exercising an instrumented build and not a stale one. + $deps = & "$env:ASAN_BIN_DIR\dumpbin.exe" /dependents "$dir\$t.exe" | Out-String + if ($deps -notmatch "clang_rt.asan") { throw "$t is not linked against the ASan runtime" } + Write-Host "$t links the ASan runtime:" + ($deps -split "`n" | Select-String "asan").Line.Trim() | ForEach-Object { Write-Host " $_" } + Copy-Item $dll $dir + & "$dir\$t.exe" + if ($LASTEXITCODE -ne 0) { throw "$t failed under ASAN (exit $LASTEXITCODE)" } + } + diff --git a/src/wolfterm.c b/src/wolfterm.c index 6b507b152..7ddd5d117 100644 --- a/src/wolfterm.c +++ b/src/wolfterm.c @@ -356,6 +356,15 @@ static int getArgs(byte* buf, word32 bufSz, word32* idx, word32* out) static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, word32 bufSz, word32* idx) { + /* A truncated OSC sequence is dropped rather than resumed: OSC state is + * not saved to escBuf and escState is never set to WS_ESC_OSC, so there is + * no resume path. Returning WS_SUCCESS lets the caller advance past the + * sequence and reset escState cleanly. */ + if (*idx >= bufSz) { + /* missing the OSC command byte, drop the sequence */ + return WS_SUCCESS; + } + if (buf[*idx] == 'P') { /* color pallet */ WLOG(WS_LOG_DEBUG, "Color pallet not yet supported"); return WS_SUCCESS; @@ -375,6 +384,9 @@ static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, word32 i; *idx += 1; + if (*idx >= bufSz) { + break; /* truncated sequence, drop it */ + } if (buf[*idx] == ';') { *idx += 1; } @@ -385,8 +397,11 @@ static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf, /* BEL (0x07) ends string */ i = *idx; while (i < bufSz && buf[i] != 0x07) i++; - if (buf[i] != 0x07) { - break; /*bell not found, possible want read? */ + if (i >= bufSz) { + /* bell not found, consume and drop the truncated sequence so + * the partial title is not emitted as raw output */ + *idx = bufSz; + break; } /* sanity check on underflow */ diff --git a/tests/api.c b/tests/api.c index 5131a7721..2a0723107 100644 --- a/tests/api.c +++ b/tests/api.c @@ -2127,6 +2127,22 @@ static byte color_test[] = { 0x1B, 0x5B, 0x34, 0x39, 0x6D, 0x64, 0x65, 0x66, 0x61, 0x75, 0x6C, 0x74, 0x20, 0x62, 0x67, 0x0A, }; + +/* OSC (ESC ]) sequences that end before the command is complete. These + * exercise the bounds checks in wolfSSH_DoOSC; each truncated sequence is + * dropped (WS_SUCCESS) without reading past the end of the buffer. */ +static byte osc_trunc_cmd[] = { /* ends right after "ESC ]" */ + 0x1B, 0x5D +}; +static byte osc_trunc_arg[] = { /* ends after "ESC ] 0" */ + 0x1B, 0x5D, 0x30 +}; +static byte osc_trunc_str[] = { /* "ESC ] 0 ; title" with no BEL terminator */ + 0x1B, 0x5D, 0x30, 0x3B, 0x74, 0x69, 0x74, 0x6C, 0x65 +}; +static byte osc_full[] = { /* well formed "ESC ] 0 ; hi BEL" */ + 0x1B, 0x5D, 0x30, 0x3B, 0x68, 0x69, 0x07 +}; #endif /* USE_WINDOWS_API */ @@ -2163,6 +2179,17 @@ static void test_wolfSSH_ConvertConsole(void) AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, color_test, 1), WS_SUCCESS); /* should skip over unknown console code */ + /* truncated OSC sequences must be dropped without an out of bounds read */ + AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_cmd, + sizeof(osc_trunc_cmd)), WS_SUCCESS); + AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_arg, + sizeof(osc_trunc_arg)), WS_SUCCESS); + AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_str, + sizeof(osc_trunc_str)), WS_SUCCESS); + /* a well formed OSC sequence still parses */ + AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_full, + sizeof(osc_full)), WS_SUCCESS); + wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); #endif /* USE_WINDOWS_API */