Repository navigation
Fix potential overflow issues in WPF native code - #11642
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple potential integer-overflow and out-of-bounds scenarios across WPF native components (WpfGfx, PenImc, DirectWriteForwarder/TtfDelta), and introduces AppContext-controlled “opt-out” switches to preserve backward behavior when required.
Changes:
- Adds overflow-safe arithmetic and additional bounds validations around texture sizing, buffer offsets, and palette indexing in native code paths.
- Introduces new native/managed switches (WpfGfx, PenImc, TtfDelta) to disable the added bounds-check protections if necessary.
- Hardens the DirectWriteForwarder TrueType subsetter (TtfDelta) with widespread allocation/offset overflow checks guarded by the new switch.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/resources/glyphrunslave.cpp | Adds overflow-checked texture size computations for glyph alpha map realization. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/hw/hwbitmapcolorsource.cpp | Uses overflow-safe offset math and buffer-size validation before CopyPixels/self-copy operations. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/hw/d3dglyphbank.cpp | Uses 64-bit arithmetic for src pointer/row calculations to avoid overflow. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/dll/wpfgfx.def | Exports a new native entry point to control bounds-check protection. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/common/gradienttexture.cpp | Whitespace-only change. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/core/common/engine.cpp | Removes trailing blank lines. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/shared/WpfGfxSwitches.h | Adds a native switch holder for WpfGfx bounds-check mitigation (missing standard license header). |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/shared/WpfGfxSwitches.cpp | Implements WpfGfx switch storage and exported setter. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/shared/shared.vcxproj | Includes WpfGfxSwitches.cpp in the shared project build. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/shared/shared.h | Ensures WpfGfxSwitches.h is included for consumers. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/shared/dynarray.h | Uses FreAssert vs Assert in First/Last based on the WpfGfx mitigation switch. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/scanop/soconvert.cpp | Adds palette bounds protections gated by the WpfGfx mitigation switch. |
| src/Microsoft.DotNet.Wpf/src/WpfGfx/common/effects/effectlist.cpp | Adds offset+size bounds validation when returning effect parameter blocks. |
| src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/UnsafeNativeMethodsMilCoreApi.cs | Adds P/Invoke for WpfGfx bounds-check switch setter. |
| src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/MediaSystem.cs | Passes the WpfGfx bounds-check mitigation switch to native at startup. |
| src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/Win32/UnsafeNativeMethodsPenimc.cs | Adds PenImc P/Invoke and forwards AppContext mitigation switch during activation. |
| src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/CoreAppContextSwitches.cs | Adds AppContext switch definitions for DWF, PenImc, and WpfGfx mitigations. |
| src/Microsoft.DotNet.Wpf/src/PenImc/dll/PimcTablet.cpp | Adds upper bounds on COM-reported counts; adds null-guard in IsPropertySupported when mitigation enabled. |
| src/Microsoft.DotNet.Wpf/src/PenImc/dll/PimcContext.cpp | Uses overflow-safe doubling for packet buffer sizing when mitigation enabled. |
| src/Microsoft.DotNet.Wpf/src/PenImc/dll/PenImcSwitches.h | Adds a native switch holder for PenImc bounds-check mitigation (missing standard license header). |
| src/Microsoft.DotNet.Wpf/src/PenImc/dll/PenImc.def | Exports PenImc switch setter (PRIVATE). |
| src/Microsoft.DotNet.Wpf/src/PenImc/dll/PenImc.cpp | Implements PenImc switch storage and exported setter. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/main.cpp | Defines the DWF/TtfDelta global “safe checks enabled” flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/ttftable.cpp | Adds allocation overflow checks and additional bounds checks (incl. negative index check) behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/ttftabl1.cpp | Adds overflow checks for read offsets behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/ttfdelta.cpp | Adds overflow-safe size calculations; changes CalcOutputBufferSize to return error code when safety enabled. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/ttfacc.cpp | Adds parameter validation and overflow-safe offset/size computations behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/ttf_safe_checks.h | Introduces the DWF/TtfDelta safety flag helper macro and extern. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/modtable.cpp | Adds overflow-safe allocations and offset checks behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/modsbit.cpp | Adds extensive EBDT bounds checking and safer realloc/offset math behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/modglyf.cpp | Adds overflow-safe allocation and offset computations for glyf/loca operations behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/modcmap.cpp | Adds overflow-safe allocations and guards against empty glyph-map access when safety enabled. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/makeglst.cpp | Adds overflow-safe allocations for symbol list/loca/components behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/intsafe_private_copy.h | Adds uint32 convenience wrappers for overflow-safe helpers. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/TtfDelta/automap.cpp | Adds overflow checks and bounds validation for GSUB/JSTF parsing behind the safety flag. |
| src/Microsoft.DotNet.Wpf/src/DirectWriteForwarder/CPP/TrueTypeSubsetter/truetype.cpp | Initializes the safety flag from AppContext and adds size validations before allocating managed arrays. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dipeshmsft
left a comment
There was a problem hiding this comment.
Ran the script to verify the changes with fixes in 8.0 and 9.0. There are no extra changes. LGTM.
|
This PR causes a functional regression: Filed as #11834 with a minimal repro and a #11834 also documents that |
|
Following up on my comment above with the root cause, and a second affected platform. Recap: since this PR shipped, Root cause: the legacy Correlation across 25 fonts is exact - all 15 faces whose DirectWrite ignores the Suggested fix: an inconsistent legacy Also affects .NET Framework. The equivalent change shipped as KB5120708 on 2026-08-11 (https://support.microsoft.com/help/5120708); |
* Merged PR 65032: [release/9.0] Restrictive-XAML/XPS integrity and hyperlink navigation security fixes - **Restrictive XAML:** block untrusted URI fetches and unsafe navigation targets in RestrictiveXamlXmlReader, WpfXamlLoader, XamlReader (new RestrictiveXamlContext helper for IsUnsafe taint tracking). - **Hyperlink navigation:** propagate IsUnsafe taint through NavigationService to prevent bypass of restrictive XAML checks via hyperlink navigation. - **XPS package integrity:** enforce full element/prefix coverage and cryptographic Signature@Id binding in XpsDigitalSignature, XpsFixedDocumentReaderWriter, XpsFixedDocumentSequenceReaderWriter (new ReachAppContextSwitches for compat opt-outs). - **TrueType subsetter:** add font security protection switch (g_fFontSecurityProtectionEnabled) honored in truetype.h. - **XamlPullParser:** add a nesting-depth guard to prevent unbounded recursion. - **DefaultCredentialsZonePolicy:** stop caching positive DNS resolution decisions. - **New AppContext compatibility switches** (default off = secure): Switch.System.Windows.Markup.OptOutOfRestrictiveXamlUriBlock, Switch.System.Windows.Markup.OptOutOfNavigationTypeRestrictiveXamlBlock, Switch.System.Windows.Xps.OptOutOfPageSignatureCoverageEnforcement, Switch.System.Windows.Xps.OptOutOfSignatureIdBindingVerification, Switch.MS.Internal.TtfDelta.DisableFontSecurityProtection. ---- Security hardening release addressing multiple vulnerabilities in XAML/XPS processing, hyperlink navigation, credential leakage via DNS rebinding, and font parsing. This PR implements comprehensive security fixes for WPF 9.0, addressing critical vulnerabilities in restrictive XAML loading, XPS digital signature validation, hyperlink navigation security, DNS rebinding attacks, and font subsetting buffer overflows. - `DefaultCredentialsZonePolicy.cs`: Implements fail-closed DNS result caching (negative results only) with TTL expiration to prevent credential leakage from DNS rebinding attacks - `NavigationService.cs`: Expands restrictive XAML reader enforcement to all XPS-originated navigation and navigation from restrictively-loaded content via logical tree traversal - `XpsDigitalSignature.cs`: Adds cryptographic binding verification for Signature@Id to prevent signature spoofing attacks, with new opt-out switch - `RestrictiveXamlXmlReader.cs`: Blocks non-pack absolute URIs and UNC paths on Uri/ImageSource properties to prevent external resource fetching from untrusted XAML - `modcmap.cpp` and `mtxcalc.cpp`: Fixes heap buffer over-read vulnerabilities in TrueType cmap Format 6 processing and composite glyph parsing with bounds validation <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> * Merged PR 64002: [release/9.0] Fix pixel-shader OOB read/write in CPSTrans::Initialize Backport of the .NET 8.0 pixel-shader memory-safety fix (PR 63617, MSRC 127649 + MSRC 130198) to release/9.0. Guards pass-1/pass-2 disagreement in DEF/DEFI/DEFB and DCL handling and bounds the parameter-token skip loop in CPSTrans::Initialize, all gated behind WpfGfxSwitches::IsWpfGfxBoundsCheckProtectionDisabled(). ---- #### AI description (iteration 1) #### PR Classification Security bug fix to prevent out-of-bounds (OOB) read and write vulnerabilities in the pixel shader translation code. #### PR Summary This PR addresses security vulnerabilities (MSRC 127649, MSRC 130198) by adding bounds checking throughout the pixel shader bytecode parsing in `CPSTrans::Initialize` to prevent buffer overruns during instruction array construction and token stream processing. - Added bounds checks in the second pass to prevent instruction array overruns and ensure room for the END record - Fixed parameter token loop to validate against bytecode buffer end, preventing OOB reads when token stream lacks D3DPS_END terminator - Fixed `D3DSIO_DCL` instruction handling to always consume register tokens, ensuring first and second pass synchronization - Added default cases for constant definition instructions (`D3DSIO_DEF`, `D3DSIO_DEFI`, `D3DSIO_DEFB`) to advance token pointer consistently between passes, preventing allocation mismatches <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> * Merged PR 63406: [.NET 9.0] Fix destination-side integer overflow in CD3DGlyphBank::RectFillAlpha (MSRC 31000000667690) Fix destination-side integer overflow in CD3DGlyphBank::RectFillAlpha (MSRC 31000000667690) The hardware glyph-bank copy computed its destination row pointers using 32-bit `lockedRect.Pitch * coordinate` products. Attacker-controlled XPS Glyphs coordinates can drive `srcRect.top` / `y` into the millions, so the product exceeds INT32 range and wraps; after sign extension the destination pointer lands ~4 GiB away from the locked temporary surface. The following memset/memcpy then writes out of bounds, terminating the WPF renderer process with 0xC0000005 (denial of service). PR #11642 previously widened only the source-side products to INT64 and left the destination-side products at 32-bit. This change widens every destination `lockedRect.Pitch` product to INT64 - the `pDst00` base plus all four row loops (top edge, real data, degenerate, bottom edge) - gated behind the same WpfGfxSwitches::IsWpfGfxBoundsCheckProtectionDisabled() kill-switch used for the source side, so both halves revert together when the mitigation is disabled. Co-authored-by: Copilot ---- #### AI description (iteration 1) #### PR Classification Security bug fix addressing an integer overflow vulnerability in glyph rendering code that could lead to out-of-bounds memory writes. #### PR Summary This PR fixes a critical integer overflow vulnerability in the CD3DGlyphBank::RectFillAlpha function where attacker-controlled glyph coordinates could cause pointer arithmetic to wrap past INT32 range, resulting in out-of-bounds memory writes. - `d3dglyphbank.cpp`: Added bounds checking validation to verify the destination byte range stays within the locked temporary surface before writing - `d3dglyphbank.cpp`: Converted pointer arithmetic from 32-bit to 64-bit integers (INT64) for all `lockedRect.Pitch * y` calculations to prevent overflow when computing destination row pointers - `d3dglyphbank.cpp`: Implemented configurable protection via `WpfGfxSwitches::IsWpfGfxBoundsCheckProtectionDisabled()` to allow disabling the fix if needed while preserving legacy behavior <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> * Merged PR 64677: [.NET9.0] Fix XPS package-boundary rejection for pack:// package identities ## Summary Fixes a regression introduced by the August 2026 XPS same-package boundary enforcement (#11835). Unchanged WPF apps that register an in-memory XPS package in `PackageStore`/`XpsDocument` using a `pack://` scheme identity crash when reading the document - same-package images and fonts are wrongly rejected as "outside the current XPS package". ## Root cause `XpsLoadingContext.IsSamePackageUri` normalizes the origin package identity via `PackUriHelper.GetPackageUri(packageOrigin)`. That method's behavior depends on the identity's scheme: - Non-`pack` schemes (e.g. `xpspack://`) -> throws `ArgumentException` ("URI must contain pack:// scheme"), which the inner `catch` handled by falling back to comparing the identity as-is. - The `pack://` scheme -> `GetPackageUri` reaches `new Uri(...)` on the unescaped authority (`memorystream.xps`), and the underlying `System.Uri` constructor throws **`UriFormatException`**, which was **not** caught. The uncaught `UriFormatException` escaped to the method's outer catch, which returns `false` (fail-closed). Every same-package resource then threw `FileFormatException(Resource_XpsPackageBoundaryViolation)`, terminating the process with an unhandled exception. ## Fix Broaden the origin-normalization fallback to also catch `UriFormatException` and `InvalidOperationException`, matching the exception set the method's outer catch already anticipates. A `pack://` identity now falls back to identity comparison exactly like the working `xpspack://` path. Security is unchanged: the widened handling applies only to parsing the **origin** (the trusted package the host app opened). The **resource** URI path is untouched - scheme must be `pack`, `application:`/`siteoforigin:` authorities are rejected, and the resource's package must still `.Equals` the origin package. External (`http`/`https`/`file`/UNC) and cross-package references remain blocked. ## Testing Verified end-to-end on locally built WPF bits using the DTS repro (in-memory XPS with one image), before/after the change: | Build | Result | |-------|--------| | Unpatched | Crash `0xE0434352`, `FileFormatException` (boundary violation) | | Patched | `Completed successfully. Pages: 1`, exit 0 | Also confirmed the boundary still holds after the fix: - same-package `pack://` -> allowed - same-package `xpspack://` -> allowed - external `http://` -> blocked - cross-package `pack://` -> blocked Reproduced on .NET Framework 3.5/4.7.2/4.8/4.8.1 and .NET 8 / 9 / 10. ---- #### AI description (iteration 1) #### PR Classification Bug fix to resolve a regression in XPS package-boundary enforcement that caused print preview failures when XPS documents used bare `pack://` URIs as PackageStore identities. #### PR Summary Fixes a security boundary check that incorrectly rejected same-package XPS resources when the package identity used a bare `pack://` scheme, causing unhandled exceptions during print preview. The fix adds additional exception handlers to properly ... * Fixup test references in build * Fixing Build --------- Co-authored-by: Maria Manolova (INFRAGISTICS INC) <b-mmanolova@microsoft.com> Co-authored-by: Milko Venkov (INFRAGISTICS INC) <b-mvenkov@microsoft.com> Co-authored-by: Matt Mitchell (.NET) <mmitche@microsoft.com>
Fixes #11632 #11631
Description
Fixes the potential Overflow issues in WPF native code
Microsoft Reviewers: Open in CodeFlow