[BUG] COMException causes application to quit when plugging / unplugging Yubikey

[canton7]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

If you plug / unplug a Yubikey a lot (and YubiKeyDeviceListener has been instantiated), it's possible to occasionally hit a COMException in HidDDevice.GetFeatureReport.

This exception is thrown on a background thread, meaning that it crashes the user's application, and there is no way for the user to prevent this.

The full exception is:

System.Runtime.InteropServices.COMException: 'A device which does not exist was specified. (0x800701B1)'

The stack trace is:

System.Private.CoreLib.dll!System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(int errorCode) Line 849
Yubico.Core.dll!Yubico.PlatformInterop.HidDDevice.GetFeatureReport() Line 59
Yubico.Core.dll!Yubico.Core.Devices.Hid.WindowsHidFeatureReportConnection.GetReport() Line 34
Yubico.YubiKey.dll!Yubico.YubiKey.Pipelines.KeyboardTransform.WaitFor(System.Func<Yubico.YubiKey.KeyboardReport, bool> stopCondition, bool checkForTouch, bool shortTimeout, string timeoutMessage) Line 120
Yubico.YubiKey.dll!Yubico.YubiKey.Pipelines.KeyboardTransform.WaitForWriteResponse() Line 102
Yubico.YubiKey.dll!Yubico.YubiKey.Pipelines.KeyboardTransform.HandleSlotRequestInstruction(Yubico.Core.Iso7816.CommandApdu apdu, Yubico.YubiKey.KeyboardFrameReader frameReader, bool configInstruction) Line 64
Yubico.YubiKey.dll!Yubico.YubiKey.Pipelines.KeyboardTransform.Invoke(Yubico.Core.Iso7816.CommandApdu commandApdu, System.Type commandType, System.Type responseType) Line 46
Yubico.YubiKey.dll!Yubico.YubiKey.Pipelines.OtpErrorTransform.Invoke(Yubico.Core.Iso7816.CommandApdu command, System.Type commandType, System.Type responseType) Line 23
Yubico.YubiKey.dll!Yubico.YubiKey.KeyboardConnection.SendCommand<Yubico.YubiKey.Otp.Commands.GetDeviceInfoResponse>(Yubico.YubiKey.IYubiKeyCommand<Yubico.YubiKey.Otp.Commands.GetDeviceInfoResponse> yubiKeyCommand) Line 44
Yubico.YubiKey.dll!Yubico.YubiKey.KeyboardDeviceInfoFactory.TryGetDeviceInfoFromKeyboard(Yubico.Core.Devices.Hid.IHidDevice device, out Yubico.YubiKey.YubiKeyDeviceInfo yubiKeyDeviceInfo) Line 51
Yubico.YubiKey.dll!Yubico.YubiKey.KeyboardDeviceInfoFactory.GetDeviceInfo(Yubico.Core.Devices.Hid.IHidDevice device) Line 24
Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDevice.YubicoDeviceWithInfo.GetDeviceInfo() Line 65
Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDevice.YubicoDeviceWithInfo.YubicoDeviceWithInfo(Yubico.Core.Devices.IDevice device) Line 30
Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDeviceListener.Update() Line 128
Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDeviceListener.ListenForChanges() Line 89
System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Line 179

Poking around in a debugger, I think this exception will crash the application: there's no exception handling in YubiKeyDeviceListener.ListenForChanges. However, if you 'Continue', you hit a further exception:

System.ObjectDisposedException: 'Cannot access a disposed object. ObjectDisposed_ObjectName_Name'

Call stack:

System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 403
System.Private.CoreLib.dll!Interop.Kernel32.SetEvent(Microsoft.Win32.SafeHandles.SafeWaitHandle handle) Line 8247
System.Private.CoreLib.dll!System.Threading.EventWaitHandle.Set() Line 85
Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDeviceListener.ListenForChanges.AnonymousMethod__2(object s, Yubico.Core.Devices.Hid.HidDeviceEventArgs e) Line 77
Yubico.Core.dll!Yubico.Core.Devices.Hid.HidDeviceListener.OnArrived(Yubico.Core.Devices.Hid.IHidDevice device) Line 38
Yubico.Core.dll!Yubico.Core.Devices.Hid.WindowsHidDeviceListener.OnEventReceived(nint hNotify, nint context, Yubico.PlatformInterop.NativeMethods.CM_NOTIFY_ACTION action, nint eventDataPtr, int eventDataSize) Line 100

Here it's trying to call updateEvent.Set();, but the event has been disposed, presumably by the thread that hit the COMException returning from ListenForChanges and disposing the updateEvent on the way (but not unsubscribing from any of the _smartCardListener events).

If you 'Continue' again, you get a further exception, but I think this is just the same exception being re-thrown:

System.ObjectDisposedException: 'Cannot access a disposed object. ObjectDisposed_ObjectName_Name'

Stack trace:

[Exception] System.Private.CoreLib.dll!System.ThrowHelper.ThrowObjectDisposedException(object instance) Line 403
[Exception] System.Private.CoreLib.dll!Interop.Kernel32.SetEvent(Microsoft.Win32.SafeHandles.SafeWaitHandle handle) Line 8247
[Exception] System.Private.CoreLib.dll!System.Threading.EventWaitHandle.Set() Line 84
[Exception] Yubico.YubiKey.dll!Yubico.YubiKey.YubiKeyDeviceListener.ListenForChanges.AnonymousMethod__0(object s, Yubico.Core.Devices.SmartCard.SmartCardDeviceEventArgs e) Line 67
[Exception] Yubico.Core.dll!Yubico.Core.Devices.SmartCard.SmartCardDeviceListener.OnArrived(Yubico.Core.Devices.SmartCard.ISmartCardDevice device) Line 40
[Exception] Yubico.Core.dll!Yubico.Core.Devices.SmartCard.DesktopSmartCardDeviceListener.FireEvents(System.Collections.Generic.List<Yubico.Core.Devices.SmartCard.ISmartCardDevice> arrivedDevices, System.Collections.Generic.List<Yubico.Core.Devices.SmartCard.ISmartCardDevice> removedDevices) Line 298
[Exception] Yubico.Core.dll!Yubico.Core.Devices.SmartCard.DesktopSmartCardDeviceListener.CheckForUpdates(int timeout, bool usePnpWorkaround) Line 197
[Exception] Yubico.Core.dll!Yubico.Core.Devices.SmartCard.DesktopSmartCardDeviceListener.ListenForReaderChanges() Line 78
[Exception] System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Line 179
System.Private.CoreLib.dll!System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() Line 53
System.Private.CoreLib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Line 203

It seems that there is an attempt at error handling in YubiKeyDeviceListener.Update:

catch (Exception ex) when (ex is SCardException || ex is PlatformApiException)

But this doesn't catch the COMException that we see. Just removing the exception filter there is probably a good thing to do: then we won't crash out in a bad way if something unexpected happens.

Expected Behavior

The expected behaviour is that Yubikey does not crash the user's application in an unrecoverable way.

Steps To Reproduce

1. In a Windows project, subscribe to YubiKeyDeviceListener.Instance.Arrived. I don't think the subscription is actually necessary (just accessing YubiKeyDeviceListener.Instance might be enough), since the YuibKeyDeviceListener ctor is what starts the background thread
2. Plug and unplug the Yubikey a lot (might take a number of minutes). Vary the amount of time that the Yubikey is plugged in for, but no need to have it plugged in for more than 2s. Having Yubico Authenticator open at the same time might help reproduce, but I have seen it with nothing else open
3. Watch your application crash with the above exception

Version

1.9.0

Version

5.4.3

Anything else?

No response

[DennisDyallo]

Hi again! Are you able to test this on the 1.11 release?

[canton7]

Yes, I can still replicate this on 1.11.

This perhaps isn't surprising, as it doesn't look like any of the files in the stack trace have been modified in any meaningful way?