Skip to content

Commit

Permalink
Add rust module to prevent run-time memory dumping of main (#9393)
Browse files Browse the repository at this point in the history
  • Loading branch information
quexten authored Aug 5, 2024
1 parent 2ea691e commit 8090a89
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 4 deletions.
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ arboard = { version = "=3.4.0", default-features = false, features = [
] }
base64 = "=0.22.1"
cbc = { version = "=0.1.2", features = ["alloc"] }
libc = "0.2.155"
rand = "=0.8.5"
retry = "=2.0.0"
scopeguard = "=1.2.0"
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/desktop_native/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ pub mod clipboard;
pub mod crypto;
pub mod error;
pub mod password;
pub mod process_isolation;
pub mod powermonitor;
51 changes: 51 additions & 0 deletions apps/desktop/desktop_native/core/src/process_isolation/linux.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use anyhow::Result;
use libc::{c_int, self};
#[cfg(target_env = "gnu")]
use libc::c_uint;

// RLIMIT_CORE is the maximum size of a core dump file. Setting both to 0 disables core dumps, on crashes
// https://github.com/torvalds/linux/blob/1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0/include/uapi/asm-generic/resource.h#L20
#[cfg(target_env = "musl")]
const RLIMIT_CORE: c_int = 4;
#[cfg(target_env = "gnu")]
const RLIMIT_CORE: c_uint = 4;

// PR_SET_DUMPABLE makes it so no other running process (root or same user) can dump the memory of this process
// or attach a debugger to it.
// https://github.com/torvalds/linux/blob/a38297e3fb012ddfa7ce0321a7e5a8daeb1872b6/include/uapi/linux/prctl.h#L14
const PR_SET_DUMPABLE: c_int = 4;

pub fn disable_coredumps() -> Result<()> {
let rlimit = libc::rlimit {
rlim_cur: 0,
rlim_max: 0,
};
if unsafe { libc::setrlimit(RLIMIT_CORE, &rlimit) } != 0 {
let e = std::io::Error::last_os_error();
return Err(anyhow::anyhow!("failed to disable core dumping, memory might be persisted to disk on crashes {}", e))
}

Ok(())
}

pub fn is_core_dumping_disabled() -> Result<bool> {
let mut rlimit = libc::rlimit {
rlim_cur: 0,
rlim_max: 0,
};
if unsafe { libc::getrlimit(RLIMIT_CORE, &mut rlimit) } != 0 {
let e = std::io::Error::last_os_error();
return Err(anyhow::anyhow!("failed to get core dump limit {}", e))
}

Ok(rlimit.rlim_cur == 0 && rlimit.rlim_max == 0)
}

pub fn disable_memory_access() -> Result<()> {
if unsafe { libc::prctl(PR_SET_DUMPABLE, 0) } != 0 {
let e = std::io::Error::last_os_error();
return Err(anyhow::anyhow!("failed to disable memory dumping, memory is dumpable by other processes {}", e))
}

Ok(())
}
13 changes: 13 additions & 0 deletions apps/desktop/desktop_native/core/src/process_isolation/macos.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use anyhow::{bail, Result};

pub fn disable_coredumps() -> Result<()> {
bail!("Not implemented on Mac")
}

pub fn is_core_dumping_disabled() -> Result<bool> {
bail!("Not implemented on Mac")
}

pub fn disable_memory_access() -> Result<()> {
bail!("Not implemented on Mac")
}
5 changes: 5 additions & 0 deletions apps/desktop/desktop_native/core/src/process_isolation/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[cfg_attr(target_os = "linux", path = "linux.rs")]
#[cfg_attr(target_os = "windows", path = "windows.rs")]
#[cfg_attr(target_os = "macos", path = "macos.rs")]
mod process_isolation;
pub use process_isolation::*;
13 changes: 13 additions & 0 deletions apps/desktop/desktop_native/core/src/process_isolation/windows.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use anyhow::{bail, Result};

pub fn disable_coredumps() -> Result<()> {
bail!("Not implemented on Windows")
}

pub fn is_core_dumping_disabled() -> Result<bool> {
bail!("Not implemented on Windows")
}

pub fn disable_memory_access() -> Result<()> {
bail!("Not implemented on Windows")
}
6 changes: 6 additions & 0 deletions apps/desktop/desktop_native/napi/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ export namespace clipboards {
export function read(): Promise<string>
export function write(text: string, password: boolean): Promise<void>
}
export namespace processisolations {
export function disableCoredumps(): Promise<void>
export function isCoreDumpingDisabled(): Promise<boolean>
export function disableMemoryAccess(): Promise<void>
}

export namespace powermonitors {
export function onLock(callback: (err: Error | null, ) => any): Promise<void>
export function isLockMonitorAvailable(): Promise<boolean>
Expand Down
3 changes: 2 additions & 1 deletion apps/desktop/desktop_native/napi/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,10 @@ if (!nativeBinding) {
throw new Error(`Failed to load native binding`)
}

const { passwords, biometrics, clipboards, powermonitors } = nativeBinding
const { passwords, biometrics, clipboards, processisolations, powermonitors } = nativeBinding

module.exports.passwords = passwords
module.exports.biometrics = biometrics
module.exports.clipboards = clipboards
module.exports.processisolations = processisolations
module.exports.powermonitors = powermonitors
19 changes: 19 additions & 0 deletions apps/desktop/desktop_native/napi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,25 @@ pub mod clipboards {
}
}

#[napi]
pub mod processisolations {
#[napi]
pub async fn disable_coredumps() -> napi::Result<()> {
desktop_core::process_isolation::disable_coredumps()
.map_err(|e| napi::Error::from_reason(e.to_string()))
}
#[napi]
pub async fn is_core_dumping_disabled() -> napi::Result<bool> {
desktop_core::process_isolation::is_core_dumping_disabled()
.map_err(|e| napi::Error::from_reason(e.to_string()))
}
#[napi]
pub async fn disable_memory_access() -> napi::Result<()> {
desktop_core::process_isolation::disable_memory_access()
.map_err(|e| napi::Error::from_reason(e.to_string()))
}
}

#[napi]
pub mod powermonitors {
use napi::{threadsafe_function::{ErrorStrategy::CalleeHandled, ThreadsafeFunction, ThreadsafeFunctionCallMode}, tokio};
Expand Down
35 changes: 32 additions & 3 deletions apps/desktop/src/main/window.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { firstValueFrom } from "rxjs";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service";
import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";
import { processisolations } from "@bitwarden/desktop-napi";

import { WindowState } from "../platform/models/domain/window-state";
import { DesktopSettingsService } from "../platform/services/desktop-settings.service";
Expand All @@ -31,6 +32,7 @@ export class WindowMain {
private windowStateChangeTimer: NodeJS.Timeout;
private windowStates: { [key: string]: WindowState } = {};
private enableAlwaysOnTop = false;
private enableRendererProcessForceCrashReload = false;
session: Electron.Session;

readonly defaultWidth = 950;
Expand All @@ -53,9 +55,11 @@ export class WindowMain {
this.win.setBackgroundColor(await this.getBackgroundColor());

// By default some linux distro collect core dumps on crashes which gets written to disk.
const crashEvent = once(this.win.webContents, "render-process-gone");
this.win.webContents.forcefullyCrashRenderer();
await crashEvent;
if (this.enableRendererProcessForceCrashReload) {
const crashEvent = once(this.win.webContents, "render-process-gone");
this.win.webContents.forcefullyCrashRenderer();
await crashEvent;
}

this.win.webContents.reloadIgnoringCache();
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
Expand Down Expand Up @@ -101,6 +105,31 @@ export class WindowMain {
// initialization and is ready to create browser windows.
// Some APIs can only be used after this event occurs.
app.on("ready", async () => {
if (isMac() || isWindows()) {
this.enableRendererProcessForceCrashReload = true;
} else if (isLinux() && !isDev()) {
if (await processisolations.isCoreDumpingDisabled()) {
this.logService.info("Coredumps are disabled in renderer process");
this.enableRendererProcessForceCrashReload = true;
} else {
this.logService.info("Disabling coredumps in main process");
try {
await processisolations.disableCoredumps();
} catch (e) {
this.logService.error("Failed to disable coredumps", e);
}
}

this.logService.info(
"Disabling external memory dumps & debugger access in main process",
);
try {
await processisolations.disableMemoryAccess();
} catch (e) {
this.logService.error("Failed to disable memory access", e);
}
}

await this.createWindow();
resolve();
if (this.argvCallback != null) {
Expand Down

0 comments on commit 8090a89

Please sign in to comment.