Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Jul 15, 2025

Backport of #117596 to release/9.0-staging

/cc @AaronRobinsonMSFT

Customer Impact

  • Customer reported
  • Found internally

A customer has uncovered a degenerate case using dynamic COM support and legacy COM servers using the wCode field on EXCEPINFO.

The existing code incorrectly handles this case at the following location. If errorCode is a non-failing HRESULT, this will crash.

int errorCode = (scode != 0) ? scode : wCode;
Exception exception = Marshal.GetExceptionForHR(errorCode);
string message = ConvertAndFreeBstr(ref bstrDescription);
if (message != null)
{
// If we have a custom message, create a new Exception object with the message set correctly.
// We need to create a new object because "exception.Message" is a read-only property.
if (exception is COMException)
{
exception = new COMException(message, errorCode);
}
else
{
Type exceptionType = exception.GetType();
ConstructorInfo ctor = exceptionType.GetConstructor(new Type[] { typeof(string) });
if (ctor != null)
{
exception = (Exception)ctor.Invoke(new object[] { message });
}
}
}

Technical details

The wCode field in the EXCEPINFO is a 16-bit signed integer. This is a legacy field for 16-bit Windows, but is still used in many COM servers. The current implementation assumed it would result in an Exception using Marshal.GetExceptionForHR(), but converting a short to an int, will rarely result in a negative value which means null will almost always be returned. This change checks if the exception is null and if so, creates a simple COMException with the error code.

Regression

  • Yes
  • No

This has been a long standing issue, even existing in .NET Framework. The generally manifests as a NullReferenceException due to invalid processing of an error code. The original error code is lost. This fix will now throw an exception with the error code returned by the COM server.

Testing

A test was added and the customer verified the fix worked for them.

Risk

Low from a functional standpoint. It is Medium from a compat perspective. We will not be throwing a COMException with the error code instead of a NullReferenceException.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

The wCode field in the EXCEPINFO is a 16-bit signed integer. This
is a legacy field for 16-bit Windows, but is still used in many COM
servers. The current implementation assumed it would result in an
Exception using Marshal.GetExceptionForHR(), but converting a
short to an int, will rarely result in a negative value which means
null will almost always be returned. This change checks if the
exception is null and if so, creates a simple COMException with the
error code.
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 9.0.x milestone Jul 15, 2025
@AaronRobinsonMSFT AaronRobinsonMSFT added the Servicing-consider Issue for next servicing release review label Jul 15, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @cston
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. please get a code review. we will take for consideration in 9.0.x

@AaronRobinsonMSFT
Copy link
Member

Closing. This doesn't meet the .NET 9.0 bar for compat risk reasons at present.

@jkotas jkotas deleted the backport/pr-117596-to-release/9.0-staging branch July 20, 2025 00:00
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Dynamic.Runtime Servicing-consider Issue for next servicing release review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants