Skip to content

Reject SAFEARRAY to IntPtr[] marshalling with mismatched native element size#129444

Open
elinor-fung wants to merge 3 commits into
dotnet:mainfrom
elinor-fung:fix/safearray-intptr-size-mismatch
Open

Reject SAFEARRAY to IntPtr[] marshalling with mismatched native element size#129444
elinor-fung wants to merge 3 commits into
dotnet:mainfrom
elinor-fung:fix/safearray-intptr-size-mismatch

Conversation

@elinor-fung

@elinor-fung elinor-fung commented Jun 15, 2026

Copy link
Copy Markdown
Member

When marshalling a SAFEARRAY as a IntPtr/UIntPtr array, we could end up going past the length of the array if the element size is less than the pointer size.

Reject the element size mismatch with SafeArrayTypeMismatchException in GetInstantiatedSafeArrayMethod — the single point that instantiates the managed content converter for all SAFEARRAY marshalling paths (P/Invoke IL stubs, variant marshalling, and IDispatch parameter marshalling) — so the mismatch is detected before any data is copied.

Note

This pull request was authored with assistance from GitHub Copilot.

…nt size

A native VT_I4/VT_INT/VT_UI4/VT_UINT SAFEARRAY has 4-byte elements, but IntPtr/UIntPtr is 8 bytes in a 64-bit process. When marshalling such a SAFEARRAY to or from a managed IntPtr[]/UIntPtr[], BlittableArrayMarshaler<T> copied sizeof(IntPtr) bytes per element, which does not match the 4-byte native element size.

Reject the element size mismatch with SafeArrayTypeMismatchException in GetInstantiatedSafeArrayMethod, the single point that instantiates the content converter for all SAFEARRAY marshalling paths, so the mismatch is detected before any data is copied.

Add a regression test (gated on Is64BitProcess) that exercises both marshalling directions via deliberately mismatched [MarshalAs(SafeArray, VT_I4)] IntPtr[,] declarations of the existing SafeArrayNative entry points.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a guard in CoreCLR SAFEARRAY marshalling to reject cases where the managed element type (IntPtr/UIntPtr) implies pointer-sized copies but the native SAFEARRAY VARTYPE implies a different element size (e.g., VT_I4 = 4 bytes). It also adds a regression test to ensure the mismatch fails fast with SafeArrayTypeMismatchException.

Changes:

  • Add an element-size mismatch check for IntPtr/UIntPtr in GetInstantiatedSafeArrayMethod, throwing SafeArrayTypeMismatchException before any content copy occurs.
  • Add a 64-bit-only regression test that validates both marshalling directions throw when [MarshalAs(SafeArraySubType = VT_I4)] is applied to an IntPtr[,].
Show a summary per file
File Description
src/coreclr/vm/olevariant.cpp Reject SAFEARRAY content converter instantiation when IntPtr/UIntPtr size doesn’t match the native VARTYPE element size.
src/tests/Interop/ArrayMarshalling/SafeArray/SafeArrayTest.cs Add regression coverage for the mismatched SAFEARRAY subtype vs IntPtr element size scenario (64-bit).

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread src/tests/Interop/ArrayMarshalling/SafeArray/SafeArrayTest.cs
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 15, 2026 22:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread src/tests/Interop/ArrayMarshalling/SafeArray/SafeArrayTest.cs
// (e.g. 4 bytes for VT_I4/VT_INT/VT_UI4/VT_UINT) can differ from the pointer size (e.g. 8 bytes
// on a 64-bit process), in which case the copy length would not match the native element size.
// Reject the mismatch as a type mismatch instead of performing a mismatched copy.
if ((thElementType == TypeHandle(CoreLibBinder::GetClass(CLASS__INTPTR)) || thElementType == TypeHandle(CoreLibBinder::GetClass(CLASS__UINTPTR)))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be logic in GetElementTypeForSafeArrayVarType() as well that might be able to handle this.

Follow up, is this also in .NET 8 or 9?

Comment thread src/tests/Interop/ArrayMarshalling/SafeArray/SafeArrayTest.cs
Copilot AI review requested due to automatic review settings June 15, 2026 23:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread src/tests/Interop/ArrayMarshalling/SafeArray/SafeArrayTest.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants