Repository navigation
fix: improve asar archive path handling - #55183
MarshallOfSound merged 3 commits into
Conversation
Treat an app's asar archive the same way in every process, so helper processes resolve it against the app bundle and agree with the browser process about what is at that path. Co-authored-by: Samuel Attard <[email protected]>
Make JavaScript fs lookups treat a path as an archive once it has been opened as one, even if that path was earlier seen as a directory. Co-authored-by: Samuel Attard <[email protected]>
trop
Bot
added
44-x-y
backport
The backported asar path handling calls GetResourcesPath(), which is private to node_bindings.cc on this branch. Also add the spec helpers its tests need.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Besides the inline findings, I checked whether the archive_mac.mm switch to MainApplicationBundle() could newly crash helper processes for in-bundle archives that are missing from ElectronAsarIntegrity. I ruled it out: the main process already stops on the same LOG(FATAL) in archive.cc:272-276 for such archives, so helpers now just match it.
Extended reasoning...
This backport touches ASAR path caching in asar_util.cc, macOS integrity lookup in archive_mac.mm and the GetResourcesPath export, plus new specs. ASAR integrity and fuses are a security-sensitive area. The inline findings already call for a human look, so this note only records one ruled-out candidate.
| base::AutoLock auto_lock(cache.lock); | ||
| cache.is_directory.emplace(path, info.is_directory); | ||
| return info.is_directory; |
There was a problem hiding this comment.
🟡 (optional) A concurrent probe can still read a swapped-in directory in place of an archive that was just opened. That is the case this change is meant to close. The miss at line 78 and the disk probe at line 94 happen without the lock. If another thread opens the archive in between, emplace at line 101 does nothing, yet line 102 still returns the stale info.is_directory. Fix: return the value actually stored, e.g. auto [it, inserted] = cache.is_directory.emplace(path, info.is_directory); return it->second;, so the false that GetOrCreateAsarArchive() writes with insert_or_assign always wins.
Why this was flagged
Thread A calls IsDirectoryCached() from GetAsarArchivePath() or IsArchivePrefix(). It misses the cache at asar_util.cc:78 and releases the lock. Thread B then runs GetOrCreateAsarArchive(), opens the archive and records insert_or_assign(path, false) at asar_util.cc:134. Next, the file is replaced by a directory, and A's base::GetFileInfo at asar_util.cc:94 sees that directory. The emplace at asar_util.cc:101 is a no-op because B's entry already exists, but asar_util.cc:102 returns info.is_directory (true) rather than the cached false. A's caller then treats the path as a plain directory and reads the attacker's file through the real filesystem, the very swap the new comment at lines 130-131 says must not be read. Possible callers include Node worker threads and the browser's asar_url_loader. The cache itself stays correct, so only that one racing lookup is affected; the base branch had no such invariant to break. Nothing in the diff guards against this: the lock is not held across the probe, and emplace never overwrites.
Verification: In shell/common/asar/asar_util.cc, line 94 probes the disk without the lock, emplace at line 101 does nothing when GetOrCreateAsarArchive() has already stored false, and line 102 still returns true. The base branch already read a swapped-in directory on every call, so this is a gap in the new hardening, not a regression.
| bool IsRequiredAppArchive(const base::FilePath& path) { | ||
| if (!electron::fuses::IsOnlyLoadAppFromAsarEnabled()) | ||
| return false; | ||
| static const base::NoDestructor<std::pair<base::FilePath, base::FilePath>> | ||
| s_paths([] { | ||
| electron::ScopedAllowBlockingForElectron allow_blocking; | ||
| const base::FilePath resources = electron::GetResourcesPath(); | ||
| const base::FilePath name(FILE_PATH_LITERAL("app.asar")); | ||
| return std::make_pair( | ||
| resources.Append(name), | ||
| base::MakeAbsoluteFilePath(resources).Append(name)); | ||
| }()); | ||
| return path == s_paths->first || path == s_paths->second; | ||
| } |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: Pre-existing, security-relevant: archives listed in ElectronAsarIntegrity other than app.asar can still be swapped for a directory, and that directory is read without integrity checks. IsRequiredAppArchive() only pins <resources>/app.asar, and only when the OnlyLoadAppFromAsar fuse is on. Any other listed archive that a process has not opened yet goes through the disk probe in IsDirectoryCached(). Fix: when IsEmbeddedAsarIntegrityValidationEnabled() is on, also treat every path whose bundle-relative key is in the integrity config as an archive without asking the disk. That closes the swap for all listed archives, not just app.asar.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
The trigger is an app with the integrity fuse on whose integrity config lists more than one archive, for example Resources/other.asar next to Resources/app.asar, where the app loads code from other.asar. Someone with write access to the bundle replaces other.asar with a directory before a process opens it. This could be a helper started later or the main process itself. GetAsarArchivePath() (asar_util.cc:171) and IsArchivePrefix() (asar_util.cc:245) call IsDirectoryCached(). For this path, IsRequiredAppArchive() at asar_util.cc:54-67 returns false because it only matches app.asar. So asar_util.cc:94 probes the disk, finds a directory, and reports "not an archive". The fs wrapper then reads plain files from the directory, so header and per-file integrity checks never run. This is the directory-swap bypass this PR closes for app.asar. The base branch behaves the same for these archives. The new pin in GetOrCreateAsarArchive() (asar_util.cc:134) only helps after that process has already opened the archive.
Verification: Trigger: the EnableEmbeddedAsarIntegrityValidation fuse is on and a second integrity-listed archive is replaced with a directory before any process opens it. IsRequiredAppArchive() (asar_util.cc:54-67) returns false for every path except <resources>/app.asar. So reads fall through to the plain filesystem. Before this change, IsDirectoryCached() ran the same disk probe.
|
Release Notes Persisted
|
##### [v44.10.1](https://github.com/electron/electron/releases/tag/v44.10.1) ### Release Notes for v44.10.1 #### Fixes - Fixed DevTools "Save" prompting for a location every time instead of reusing the previously chosen file. [#55484](electron/electron#55484) - Fixed DevTools "Save As" using the old file and Linux cancels treated as saved. - Fixed `webContents.adjustSelection()` ignoring the `end` option. [#55461](electron/electron#55461) <sup>(Also in [45](electron/electron#55462), [46](https://github.com/electron/electron/pull/55463))</sup> #### Performance Improvements - Improved performance of `net.fetch()` requests without a body. [#55445](electron/electron#55445) <sup>(Also in [43](electron/electron#55447), [45](electron/electron#55446), [46](https://github.com/electron/electron/pull/55444))</sup> - Improved performance of repeated `net.fetch()` calls for `file:` URLs. [#55458](electron/electron#55458) <sup>(Also in [45](electron/electron#55459), [46](https://github.com/electron/electron/pull/55457))</sup> #### Other Changes - Backported fixes from upstream Chromium, Dawn, Skia and V8. [#55492](electron/electron#55492) ##### [v44.10.0](https://github.com/electron/electron/releases/tag/v44.10.0) ### Release Notes for v44.10.0 #### Features - Added filtering options to `session.registerPreloadScript` - `matches`, `excludeMatches`, `includeGlobs`, `excludeGlobs`, `matchOriginAsFallback`. [#55182](electron/electron#55182) <sup>(Also in [45](https://github.com/electron/electron/pull/55181))</sup> #### Fixes - Fixed a crash on Linux when a D-Bus method call was made on a disconnected bus. [#55290](electron/electron#55290) <sup>(Also in [45](electron/electron#55292), [46](https://github.com/electron/electron/pull/55289))</sup> - Fixed a main process crash when `onSendHeaders` observed Blob uploads. [#55168](electron/electron#55168) <sup>(Also in [45](https://github.com/electron/electron/pull/55169))</sup> - Fixed `app.getLoginItemSettings()` not finding login items on Windows when the executable path contains spaces. [#55207](electron/electron#55207) <sup>(Also in [45](https://github.com/electron/electron/pull/54419))</sup> - Fixed Europe/London and Europe/Dublin time zone names showing as "GMT+00:00" in winter by updating ICU to tzdata 2026e. [#55335](electron/electron#55335) - Fixed File System Access blocklist gaps for `.git/hooks` writes and hypervisor UNC shares. [#55330](electron/electron#55330) - Fixed handling of malformed asar archives whose size fields do not match the file length. [#55176](electron/electron#55176) <sup>(Also in [43](electron/electron#55179), [45](https://github.com/electron/electron/pull/55175))</sup> - Fixed inconsistent handling of asar archive paths between the main process and child processes. [#55183](electron/electron#55183) <sup>(Also in [43](electron/electron#55185), [45](https://github.com/electron/electron/pull/55184))</sup> - Fixed `Menu.popup()` on macOS sometimes not showing a menu after another closed. [#55325](electron/electron#55325) <sup>(Also in [45](electron/electron#55326), [46](https://github.com/electron/electron/pull/55324))</sup> - Fixed MV3 extension events not arriving to background service workers. [#55152](electron/electron#55152) <sup>(Also in [43](electron/electron#55153), [45](https://github.com/electron/electron/pull/55154))</sup> - Fixed paste hanging indefinitely on Linux X11 until the app was restarted. [#55327](electron/electron#55327) - Fixed `protocol.registerSource` schemes not applying the same fetch rules as `protocol.handle` schemes. [#55177](electron/electron#55177) <sup>(Also in [45](https://github.com/electron/electron/pull/55178))</sup> - Fixed `utilityProcess` not emitting an `exit` event when the app quit while the process was still launching. [#55273](electron/electron#55273) <sup>(Also in [43](electron/electron#55272), [45](electron/electron#55274), [46](https://github.com/electron/electron/pull/55264))</sup> #### Other Changes - Updated Chromium to 152.0.7977.169. [#55033](electron/electron#55033) - Updated Node.js to v24.22.1. [#55162](electron/electron#55162) ##### [v44.9.3](https://github.com/electron/electron/releases/tag/v44.9.3) ### Release Notes for v44.9.3 #### Other Changes - Backported fixes from upstream ANGLE, Chromium, PDFium, Skia and V8. [#55149](electron/electron#55149) ##### [v44.9.2](https://github.com/electron/electron/releases/tag/v44.9.2) ### Release Notes for v44.9.2 ##### [v44.9.1](https://github.com/electron/electron/releases/tag/v44.9.1) ### Release Notes for v44.9.1 > \[!NOTE] > This release contains the fix for <GHSA-jr5w-w6x2-84p9>, the mitigation for compromised eMudhra intermediate certificates ##### [v44.9.0](https://github.com/electron/electron/releases/tag/v44.9.0) ### Release Notes for v44.9.0 #### Features - Added app shims on macOS to show windows under another app's Dock icon. [#55073](electron/electron#55073) <sup>(Also in [45](https://github.com/electron/electron/pull/55072))</sup> #### Fixes - Fixed link-opened windows not using the opener's session and `webPreferences`. [#55048](electron/electron#55048) <sup>(Also in [42](electron/electron#55050), [43](electron/electron#55049), [45](https://github.com/electron/electron/pull/55051))</sup> - Fixed `setCertificateVerifyProc` occasionally not being applied to a request issued concurrently with the first request to a host. [#55046](electron/electron#55046) <sup>(Also in [42](electron/electron#55054), [43](electron/electron#55047), [45](https://github.com/electron/electron/pull/55045))</sup> ##### [v44.8.0](https://github.com/electron/electron/releases/tag/v44.8.0) ### Release Notes for v44.8.0 #### Features - Added support for `Notification` action buttons on Linux. [#54890](electron/electron#54890) <sup>(Also in [45](https://github.com/electron/electron/pull/54889))</sup> #### Fixes - Fixed mouse cursor missing over native dialogs in certain scenarios on Windows. [#54987](electron/electron#54987) - Fixed `net.fetch` and `session.fetch` failing permanently with `net::ERR_FAILED` after the Network Service process restarts. [#54909](electron/electron#54909) <sup>(Also in [42](electron/electron#54925), [43](electron/electron#54908), [45](https://github.com/electron/electron/pull/54910))</sup> - Fixed `utilityProcess` dropping messages posted with `process.parentPort.postMessage()` right before the process exits. [#54960](electron/electron#54960) <sup>(Also in [42](electron/electron#55000), [43](electron/electron#54962), [45](https://github.com/electron/electron/pull/54956))</sup> - Fixed `win.isFullScreen()` being stale in fullscreen events on Windows. [#55009](electron/electron#55009) <sup>(Also in [45](https://github.com/electron/electron/pull/55008))</sup> #### Other Changes - Backported fixes from upstream Chromium and V8. [#55034](electron/electron#55034) - Updated Chromium to 152.0.7977.167. [#54928](electron/electron#54928) - Updated Node.js to v24.22.0. [#55004](electron/electron#55004) ##### [v44.7.2](https://github.com/electron/electron/releases/tag/v44.7.2) ### Release Notes for v44.7.2 #### Fixes - Fixed an issue where clicks posted to a background app were dropped on macOS. [#54819](electron/electron#54819) <sup>(Also in [45](https://github.com/electron/electron/pull/54818))</sup> - Fixed interrupted macOS updates leaving a broken app behind. [#54844](electron/electron#54844) <sup>(Also in [45](https://github.com/electron/electron/pull/54843))</sup> #### Other Changes - Backported fixes from upstream ANGLE, Chromium, PDFium and V8. [#54898](electron/electron#54898) #### Documentation - Documentation changes: [#54880](electron/electron#54880) ##### [v44.7.1](https://github.com/electron/electron/releases/tag/v44.7.1) ### Release Notes for v44.7.1 #### Fixes - Fixed a crash when starting without an available display server on Linux. [#54794](electron/electron#54794) <sup>(Also in [45](https://github.com/electron/electron/pull/54781))</sup> - Fixed a Linux crash creating a window with a menu bar when GTK failed to load. [#54859](electron/electron#54859) <sup>(Also in [45](https://github.com/electron/electron/pull/54846))</sup> - Fixed a `WebContentsView` that was removed with `removeChildView()` and re-added with `addChildView()` staying hidden behind its sibling views on Linux. [#54798](electron/electron#54798) - Fixed asar archives in non-ASCII folders failing to load on Linux. [#54849](electron/electron#54849) <sup>(Also in [45](https://github.com/electron/electron/pull/54848))</sup> - Fixed renderer crashes during garbage collection inside ShadowRealms. [#54865](electron/electron#54865) <sup>(Also in [45](https://github.com/electron/electron/pull/54864))</sup> #### Other Changes - Refactored mouse forwarding on Windows to fix `setIgnoreMouseEvents(true, { forward: true })` related issues. [#54802](electron/electron#54802) <sup>(Also in [45](https://github.com/electron/electron/pull/54803))</sup> - Updated Chromium to 152.0.7977.149. [#54085](electron/electron#54085) ##### [v44.7.0](https://github.com/electron/electron/releases/tag/v44.7.0) ### Release Notes for v44.7.0 #### Features - The macOS `autoUpdater` now streams update downloads to disk, resumes an interrupted download on the next check instead of starting over, and applies a binary delta when the update feed offers one for the running version. [#54204](electron/electron#54204) #### Fixes - Fixed an uncatchable TypeError in `net.fetch` when receiving response headers with non-ASCII characters, and preserved multiple Set-Cookie headers. [#54672](electron/electron#54672) #### Other Changes - Backported fixes from upstream ANGLE, Chromium, Dawn, fontconfig, Skia, V8 and WebRTC. [#54666](electron/electron#54666) - Backported fixes from upstream ANGLE, Chromium, Skia, V8 and WebRTC. [#54657](electron/electron#54657) ##### [v44.6.0](https://github.com/electron/electron/releases/tag/v44.6.0) ### Release Notes for v44.6.0 #### Features - Added the experimental `protocol.registerSource()` to serve a custom scheme from directories on disk without a JavaScript handler. [#54522](electron/electron#54522) <sup>(Also in [45](https://github.com/electron/electron/pull/54521))</sup> #### Fixes - Fixed a crash on launch on macOS when the system refused to register the app for sleep/wake notifications. [#54596](electron/electron#54596) - Fixed a crash on `navigator.usb.requestDevice()` in in-memory sessions. [#54616](electron/electron#54616) <sup>(Also in [45](https://github.com/electron/electron/pull/54615))</sup> - Fixed `webview.findInPage()` hanging when the host page also has an iframe. [#54586](electron/electron#54586) <sup>(Also in [43](electron/electron#54584), [45](https://github.com/electron/electron/pull/54585))</sup> - Updated time zone data to tzdata 2026d, including British Columbia's move to permanent UTC-7. [#54623](electron/electron#54623) #### Documentation - Documentation changes: [#54589](electron/electron#54589)
Backport of #55173
See that PR for details.
Notes: Fixed inconsistent handling of asar archive paths between the main process and child processes.