Pressing Caps Lock with electron-v76-darwin-x64 crashes Electron App with no errors

[m1n0s]

Expected Behavior

Pressing of Caps Lock shouldn't crash an electron app

Current Behavior

Pressing Caps Lock with electron-v76-darwin-x64 crashes Electron App with no errors

Steps to Reproduce (for bugs)

Not so many steps TBH. The error reproduces right after I import or require from iohook.
Then start an electron app - click on it to put window on focus and just press Caps Lock. The app will crash with no issues and logs even in debug mode.

Your Environment

• Version used: "iohook": "^0.6.5"
• Environment name and version (e.g. Chrome 39, node.js 5.4): "electron": "^8.2.0", nodejs: 12.16.1
• Operating System and version (desktop or mobile): MacOS Catalina 10.15.4

From my own debugging I found out that issue is definitely within native code somewhere:

1. As only import is enough to reproduce the issue - I blame IOHook constructor code at line this.load(). Once I commented out code in load method app wasn't crash anymore but hooks didn't work either.
2. I was trying to play within this._handler method and place next code inside at the very beginning of the function to not perform any operation if it was Caps Lock.

if (msg.keyboard && msg.keyboard.keycode === 58) {
      console.log('CAPS LOCK PRESSED');
      return;
    }

I saw the console printed CAPS LOCK PRESSED but app failed anyway so it's not because of handler logic.

So that's why I decided it is somewhere in native code and just for MacOS as I wasn't able to reproduce the issue on Windows.

I'd be happy to hear from you guys and what I can do more in order to have further steps.
Thank you.

[marcelblum]

FWIW I can't reproduce, tested on latest Catalina 10.15.5 as well as High Sierra 10.13.6 in a packaged app using iohook 0.6.6, electron 8.4.0, node 12.13.0. Caps lock press causes no ill effect and keycodes seem to be detected the same whether caps lock on or off.

[m1n0s]

Hey @marcelblum! Thanks for such a quick response!
I'll give another try for me to create a small repo with reproducible issue in it and get back here.

[defano]

I've created a test case using one of Electron's "Simple Samples". To reproduce:

1. git clone git@github.com:defano/hash-iohook.git
2. yarn
3. yarn start
4. After app has opened, press caps lock and it will crash. Seems the same issue exists with a few other keys, too. For example, pressing the mute key (F10) will also crash the app (on my Apple Magic Keyboard with Numeric Keypad).

As noted in package.json this is based on Electron 8.4.0 and iohook 0.6.6. I am running macOS Catalina 10.15.5.

This app is identical to the one posted here, https://www.electronjs.org/blog/simple-samples, except:

1. I added iohook as a dependency in package.json
2. I added const iohook = require('iohook') to app.js.

Hope this helps--

[marcelblum]

Based on recent findings from #246, the issue only applies to when iohook is used in the main process. require it in the renderer process instead (BrowserWindow must have webPreferences: {nodeIntegration: true}) and these crashes do not occur. Which is not to say that this isn't a legitimate issue, but I think for most Electron use cases exposing iohook in the renderer has many advantages anyway. Might want to change the issue title/description to reflect this.

[defano]

Thanks @marcelblum. I can confirm it doesn't crash when loaded in the renderer process. This seems to be a dup of #246.

[janakjksol]

MY OS : Catalina (10.15.7)

when focus on electron app and press caps lock then error occur.

also I'm set webPreferences: {nodeIntegration: true}) but it can't work.

so if you have any solution then please give me.

[DCzajkowski]

Together with @gtluszcz we have been investigating these crashes, as they are a deal breaker for us.

We have found, that the problem lays somewhere in this call specifically:

iohook/libuiohook/src/darwin/input_hook.c

Line 601 in 769daed

id event_data = ((id(*)(id, SEL, void(*)))objc_msgSend)((id) objc_getClass("NSEvent"), sel_registerName("eventWithCGEvent:"), event_ref);

If you call this line, it doesn't crash immediately, though. The program continues normally. After some short period of time, the app crashes.

I can only assume (with my limited macOS-APIs knowledge), that event_ref is accessed after it's destroyed. On the other hand, the program crashes with SIGILL, not SEGFAULT. 🤷‍♀️

---

I've noticed there were some changes in this function in the original repo (https://github.com/kwhat/libuiohook/blob/1.2/src/darwin/input_hook.c#L588-L592), however these changes do not fix the issue.

---

I've been able to reproduce this with:

• node v14.15.5
• Electron 11.2.3
• iohook from master at commit 2297744
• built using: node build.js --upload=false --runtime electron --version 11.2.3 --abi 85

I've used defano/hash-iohook repo from @defano to reproduce.

I can also reproduce using:

const NodeHookAddon = require('../iohook/build/Debug/iohook.node')

NodeHookAddon.startHook((event) => {
  if (event.type <= 5) {
    console.log(event)
  }
}, true)

instead of requiring the iohook npm library itself, so the problem is definitely in the original package. I would report an issue there, however this is strictly problematic with Electron, as using it in pure node does not cause any problems.

---

cc: @ash0x0 @kwhat

[kwhat]

@DCzajkowski If this is a use-after-free issue it is likely due to a thread safety problem which could explain why you are only seeing it on Electron. There is some questionable thread safety in the input_hook (see pthread_ methods) related to the main run loop context switching. Much of this process is completely undocumented by Apple and I honestly hate working with their platform so I doubt I will be putting in a whole lot of effort into tracking down this problem unless I can reliably reproduce it in a stand alone CLI program outside of Electron/Java or anything else.

[DCzajkowski]

@kwhat this is absolutely understandable! I love macOS, but working with their "documentation" for these APIs was a terrible experience for me today. I smiled a few times reading your comments in the source though 😅

unless I can reliably reproduce it in a stand alone CLI program outside of Electron

Unfortunately, I didn't manage to attach a debugger to the Electron process that would support native code debugging, if that's what you are after. To be honest I have no clue how to reproduce it in pure Node.

Much of this process is completely undocumented by Apple

From what I've read, the source of all evil is the objc_msgSend function (https://stackoverflow.com/a/17263420). Do you think there is a way not to use it?

[marcelblum]

@DCzajkowski as I noted above a while back, the crash does not happen if you load iohook in the renderer process. I just tested it some more today after reading your post, left my packaged electron app open for several hours, hitting caps lock several times throughout, and no crash or misbehavior of any kind.

[DCzajkowski]

@marcelblum Our use-case is to recognize key-presses in the system, not inside the Electron window. We even "disable" keyboard recognition when the Electron window is focused! This is because our app provides global, system-wide shortcut recognition (see https://github.com/quickwords/quickwords as an example of we are trying to achieve in our app).

@kwhat something I forgot to mention. This happens only when the Chromium window of our Electron app is focused. In other cases, crashes do not happen.

[marcelblum]

@DCzajkowski iohook can still detect global key presses even if you instantiate it in the renderer. And you could use BrowserWindow.isFocused() in the key handling logic (or track window.onfocus/window.onblur).

[DCzajkowski]

@marcelblum I didn't know that. However, we are destroying all windows when they are closed. You know, we can fix this issue on our end by stopping/starting on focus/blur. We can also fork iohook and remove this "system" function entirely. However, this wouldn't be a fix for this #issue in general. :/

[kwhat]

@DCzajkowski,

From what I've read, the source of all evil is the objc_msgSend function (https://stackoverflow.com/a/17263420). Do you think there is a way not to use it?

I agree that it is evil and really difficult to use. IIRC, I needed to use it because there was no C API for detecting CapsLock and some of the other NX_SYSDEFINED key information. I think it was this: https://developer.apple.com/documentation/appkit/nsevent/1528289-data1

It has been a very long time since I wrote it but there are some hacks like the following to try and work around using objc_msgSend. See: https://github.com/kwhat/libuiohook/blob/1.2/src/darwin/input_hook.c#L620-L625

        // FIXME We shouldn't be doing this.
        #ifdef __LP64__
        long data = CFSwapInt64BigToHost(*((UInt64 *) (buffer + 4)));
        #else
        long data = CFSwapInt32BigToHost(*((UInt32 *) (buffer + 4)));
        #endif

I will try to duplicate it witch chrome running and see if I can get it to work. This code is in desperate need of a refactoring.

You can always try building libuiohook without USE_OBJC.

[kwhat]

Well, I may have found a solution but I cant get it to compile locally because CMake suddenly cannot find Java despite my JAVA_HOME variable pointing right at it. This is what I waste most of my free time dealing with, not fixing bugs but fighting with sub-par tooling.

[kwhat]

I have indeed found a solution, it is going to take some time to fully implement but it stops the segfault for me in Java.

[kwhat]

Alright, I had some unexpected free time on Monday / Tuesday and was able to get a fix in PR #108. I still need to do some more work fixing up some of the per-processor defines and other minor things, but it seems pretty stable. I am not sure if this library will be migrating to my upstream or continue the fork. If you are going to fork, the lines of interest are R476-R560 and possibly a few more bits near the top of the file.

The problem: Turns out this was related to the same random crashes the TIS keycode stuff was experiencing so I have it setup to run on the main runloop as long as you are on 10.6+. Fallback is kind of hacky for 10.5, but it works, doesn't require OBJC or the main runloop.

Any code review would be appreciated. Let me know if there are any issues.

[xudaolong]

@matthewshirley Do you have time to update libuiohook to version 1.2 ? thanks.

[michelvermeulen]

Following 🙏 Same thing here and iohook can't be loaded renderer side on latest versions of Electron (non-context aware modules are not allowed on the renderer anymore). Thanks!