Skip to content

Commit 2425f7c

Browse files
yosuke-wolfsslejohnstown
yosuke-wolfssl
authored and
ejohnstown
committed
Bound OSC index before reads in wolfSSH_DoOSC
1 parent 4cf61a1 commit 2425f7c

3 files changed

Lines changed: 175 additions & 2 deletions

File tree

‎.github/workflows/windows-check.yml‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,134 @@ jobs:
6565
# See https://docs.microsoft.com/visualstudio/msbuild/msbuild-command-line-reference
6666
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}}
6767

68+
# Build and run the self-contained unit tests with the MSVC AddressSanitizer.
69+
# This is the only job that executes wolfSSH tests under a sanitizer on
70+
# Windows, where the USE_WINDOWS_API console code (e.g. wolfSSH_DoOSC) is
71+
# compiled, so it guards that path against out-of-bounds reads.
72+
asan-tests:
73+
runs-on: windows-latest
74+
75+
env:
76+
# print_stats=1 emits allocator stats at exit, positive evidence that the
77+
# ASan runtime was active in-process during each test run.
78+
ASAN_OPTIONS: abort_on_error=1:print_stats=1
79+
80+
steps:
81+
- uses: actions/checkout@v2
82+
with:
83+
repository: wolfssl/wolfssl
84+
path: wolfssl
85+
86+
- uses: actions/checkout@master
87+
with:
88+
path: wolfssh
89+
90+
- name: Add MSBuild to PATH
91+
uses: microsoft/setup-msbuild@v1
92+
93+
# The pinned v142 toolset does not ship the AddressSanitizer runtime libs on
94+
# the runner. Find an installed MSVC toolset that does and build everything in
95+
# this job with it, recording its lib/bin dirs for the link and run steps.
96+
# wolfssl, wolfssh and the test projects must share one compiler version
97+
# because /GL (whole program optimization) does code generation at link time.
98+
- name: Locate MSVC AddressSanitizer runtime
99+
shell: pwsh
100+
run: |
101+
$vs = & "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath
102+
$lib = Get-ChildItem "$vs\VC\Tools\MSVC\*\lib\x64\clang_rt.asan_dynamic_runtime_thunk-x86_64.lib" -ErrorAction SilentlyContinue |
103+
Sort-Object { [version]($_.FullName -replace '.*\\MSVC\\([0-9.]+)\\.*','$1') } |
104+
Select-Object -Last 1
105+
if ($null -eq $lib) { throw "MSVC AddressSanitizer runtime not found in any installed toolset" }
106+
$msvcDir = $lib.Directory.Parent.Parent.FullName
107+
$ver = [version]($msvcDir.Split('\')[-1])
108+
if ($ver.Minor -ge 30) { $toolset = "v143" } else { $toolset = "v142" }
109+
"ASAN_TOOLSET=$toolset" | Out-File $env:GITHUB_ENV -Append
110+
"ASAN_LIB_DIR=$($lib.Directory.FullName)" | Out-File $env:GITHUB_ENV -Append
111+
"ASAN_BIN_DIR=$(Join-Path $msvcDir 'bin\Hostx64\x64')" | Out-File $env:GITHUB_ENV -Append
112+
Write-Host "Using toolset $toolset (MSVC $ver)"
113+
114+
- name: Restore wolfSSL NuGet packages
115+
working-directory: wolfssl
116+
run: nuget restore ${{env.WOLFSSL_SOLUTION_FILE_PATH}}
117+
118+
# These env paths already include the wolfssh/ and wolfssl/ checkout
119+
# prefixes, so they must run from the workspace root (as the build job does).
120+
- name: updated user_settings.h for sshd and x509
121+
run: cp ${{env.USER_SETTINGS_H_NEW}} ${{env.USER_SETTINGS_H}}
122+
123+
- name: replace wolfSSL user_settings.h with wolfSSH user_settings.h
124+
run: get-content ${{env.USER_SETTINGS_H_NEW}} | %{$_ -replace "if 0","if 1"}
125+
126+
# WholeProgramOptimization=false disables /GL so the v142-independent objects
127+
# link without a cross-version code-generation requirement (C1047).
128+
- name: Build wolfssl library
129+
working-directory: wolfssl
130+
shell: pwsh
131+
run: |
132+
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}}
133+
if ($LASTEXITCODE -ne 0) { throw "wolfssl build failed" }
134+
135+
- name: Restore NuGet packages
136+
working-directory: wolfssh\ide\winvs
137+
run: nuget restore ${{env.SOLUTION_FILE_PATH}}
138+
139+
# EnableASAN=true adds /fsanitize=address to each test project and to the
140+
# referenced wolfssh library project, instrumenting src/wolfterm.c. The
141+
# AddressSanitizer lib dir is added to LIB so the runtime thunk resolves.
142+
- name: Build api-test and unit-test with AddressSanitizer
143+
working-directory: wolfssh\ide\winvs
144+
shell: pwsh
145+
run: |
146+
$env:LIB = "$env:ASAN_LIB_DIR;$env:LIB"
147+
foreach ($p in @("api-test\api-test.vcxproj", "unit-test\unit-test.vcxproj")) {
148+
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
149+
if ($LASTEXITCODE -ne 0) { throw "build failed: $p" }
150+
}
151+
152+
# Positive control: compile a deliberate heap overflow with the same toolset
153+
# and confirm ASan aborts on it. This proves ASan detection actually works on
154+
# this runner, so a clean test run is meaningful rather than a silent no-op.
155+
- name: Verify AddressSanitizer detection (positive control)
156+
shell: pwsh
157+
run: |
158+
$vs = & "C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -latest -property installationPath
159+
Import-Module "$vs\Common7\Tools\Microsoft.VisualStudio.DevShell.dll"
160+
Enter-VsDevShell -VsInstallPath $vs -SkipAutomaticLocation -DevCmdArguments "-arch=x64" | Out-Null
161+
Set-Content canary.c 'int main(int argc, char** argv) { volatile char b[1]; (void)argv; return b[argc + 10]; }'
162+
cl /nologo /fsanitize=address /MD canary.c
163+
if ($LASTEXITCODE -ne 0) { throw "canary failed to compile" }
164+
$out = & .\canary.exe 2>&1 | Out-String
165+
$code = $LASTEXITCODE
166+
Write-Host $out
167+
if ($code -eq 0 -or $out -notmatch "AddressSanitizer") {
168+
throw "Positive control FAILED: ASan did not detect the deliberate overflow"
169+
}
170+
Write-Host "Positive control OK: ASan detected the deliberate out-of-bounds access"
171+
# canary.exe aborts with a non-zero code by design; reset so the step
172+
# itself reports success.
173+
exit 0
174+
175+
# Building a .vcxproj directly leaves $(SolutionDir) unset, so each project
176+
# writes to its own <project>\Release\x64 dir. Run from the wolfssh repo root
177+
# so the tests find keys/ and certs/, and copy the dynamic ASAN runtime next
178+
# to each binary so it loads without a full developer-prompt environment.
179+
- name: Run tests under AddressSanitizer
180+
working-directory: wolfssh
181+
shell: pwsh
182+
run: |
183+
$cfg = "${{env.WOLFSSH_BUILD_CONFIGURATION}}\${{env.BUILD_PLATFORM}}"
184+
$dll = Join-Path $env:ASAN_BIN_DIR "clang_rt.asan_dynamic-x86_64.dll"
185+
if (-not (Test-Path $dll)) { throw "ASAN runtime DLL not found: $dll" }
186+
foreach ($t in @("api-test", "unit-test")) {
187+
$dir = "ide\winvs\$t\$cfg"
188+
# Static signal: confirm the binary really imports the ASan runtime, so
189+
# we are exercising an instrumented build and not a stale one.
190+
$deps = & "$env:ASAN_BIN_DIR\dumpbin.exe" /dependents "$dir\$t.exe" | Out-String
191+
if ($deps -notmatch "clang_rt.asan") { throw "$t is not linked against the ASan runtime" }
192+
Write-Host "$t links the ASan runtime:"
193+
($deps -split "`n" | Select-String "asan").Line.Trim() | ForEach-Object { Write-Host " $_" }
194+
Copy-Item $dll $dir
195+
& "$dir\$t.exe"
196+
if ($LASTEXITCODE -ne 0) { throw "$t failed under ASAN (exit $LASTEXITCODE)" }
197+
}
198+

‎src/wolfterm.c‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,15 @@ static int getArgs(byte* buf, word32 bufSz, word32* idx, word32* out)
356356
static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
357357
word32 bufSz, word32* idx)
358358
{
359+
/* A truncated OSC sequence is dropped rather than resumed: OSC state is
360+
* not saved to escBuf and escState is never set to WS_ESC_OSC, so there is
361+
* no resume path. Returning WS_SUCCESS lets the caller advance past the
362+
* sequence and reset escState cleanly. */
363+
if (*idx >= bufSz) {
364+
/* missing the OSC command byte, drop the sequence */
365+
return WS_SUCCESS;
366+
}
367+
359368
if (buf[*idx] == 'P') { /* color pallet */
360369
WLOG(WS_LOG_DEBUG, "Color pallet not yet supported");
361370
return WS_SUCCESS;
@@ -375,6 +384,9 @@ static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
375384
word32 i;
376385

377386
*idx += 1;
387+
if (*idx >= bufSz) {
388+
break; /* truncated sequence, drop it */
389+
}
378390
if (buf[*idx] == ';') {
379391
*idx += 1;
380392
}
@@ -385,8 +397,11 @@ static int wolfSSH_DoOSC(WOLFSSH* ssh, WOLFSSH_HANDLE handle, byte* buf,
385397
/* BEL (0x07) ends string */
386398
i = *idx;
387399
while (i < bufSz && buf[i] != 0x07) i++;
388-
if (buf[i] != 0x07) {
389-
break; /*bell not found, possible want read? */
400+
if (i >= bufSz) {
401+
/* bell not found, consume and drop the truncated sequence so
402+
* the partial title is not emitted as raw output */
403+
*idx = bufSz;
404+
break;
390405
}
391406

392407
/* sanity check on underflow */

‎tests/api.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2865,6 +2865,22 @@ static byte color_test[] = {
28652865
0x1B, 0x5B, 0x34, 0x39, 0x6D, 0x64, 0x65, 0x66,
28662866
0x61, 0x75, 0x6C, 0x74, 0x20, 0x62, 0x67, 0x0A,
28672867
};
2868+
2869+
/* OSC (ESC ]) sequences that end before the command is complete. These
2870+
* exercise the bounds checks in wolfSSH_DoOSC; each truncated sequence is
2871+
* dropped (WS_SUCCESS) without reading past the end of the buffer. */
2872+
static byte osc_trunc_cmd[] = { /* ends right after "ESC ]" */
2873+
0x1B, 0x5D
2874+
};
2875+
static byte osc_trunc_arg[] = { /* ends after "ESC ] 0" */
2876+
0x1B, 0x5D, 0x30
2877+
};
2878+
static byte osc_trunc_str[] = { /* "ESC ] 0 ; title" with no BEL terminator */
2879+
0x1B, 0x5D, 0x30, 0x3B, 0x74, 0x69, 0x74, 0x6C, 0x65
2880+
};
2881+
static byte osc_full[] = { /* well formed "ESC ] 0 ; hi BEL" */
2882+
0x1B, 0x5D, 0x30, 0x3B, 0x68, 0x69, 0x07
2883+
};
28682884
#endif /* USE_WINDOWS_API */
28692885

28702886

@@ -2901,6 +2917,17 @@ static void test_wolfSSH_ConvertConsole(void)
29012917
AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, color_test, 1),
29022918
WS_SUCCESS); /* should skip over unknown console code */
29032919

2920+
/* truncated OSC sequences must be dropped without an out of bounds read */
2921+
AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_cmd,
2922+
sizeof(osc_trunc_cmd)), WS_SUCCESS);
2923+
AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_arg,
2924+
sizeof(osc_trunc_arg)), WS_SUCCESS);
2925+
AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_trunc_str,
2926+
sizeof(osc_trunc_str)), WS_SUCCESS);
2927+
/* a well formed OSC sequence still parses */
2928+
AssertIntEQ(wolfSSH_ConvertConsole(ssh, stdoutHandle, osc_full,
2929+
sizeof(osc_full)), WS_SUCCESS);
2930+
29042931
wolfSSH_free(ssh);
29052932
wolfSSH_CTX_free(ctx);
29062933
#endif /* USE_WINDOWS_API */

0 commit comments

Comments
 (0)