diff options
| author | Erik Larkö <erik@mullvad.net> | 2017-09-20 14:45:32 +0200 |
|---|---|---|
| committer | Erik Larkö <erik@mullvad.net> | 2017-09-22 08:02:55 +0200 |
| commit | f615ddacbfbbc04a2147b3ada75cefd318fd220c (patch) | |
| tree | ff83c0ff26c5a98e48ebfd41443edb8714e08ca2 /app | |
| parent | 62dde15537a2a268ccf3b967d26664466d498714 (diff) | |
| download | mullvadvpn-f615ddacbfbbc04a2147b3ada75cefd318fd220c.tar.xz mullvadvpn-f615ddacbfbbc04a2147b3ada75cefd318fd220c.zip | |
Validate the owner and permissions of the rpc address file before reading its contents
Diffstat (limited to 'app')
| -rw-r--r-- | app/main.js | 23 |
1 files changed, 23 insertions, 0 deletions
diff --git a/app/main.js b/app/main.js index 99ec3d95fa..7e69b48ad2 100644 --- a/app/main.js +++ b/app/main.js @@ -182,6 +182,17 @@ const appDelegate = { return; } + const isSecureEnough = isOwnedAndOnlyWritableByRoot(rpcAddressFile); + if (!isSecureEnough) { + log.error('Not trusting the contents of', rpcAddressFile, 'as it was not owned and only writable by root.'); + return; + } + + // There is a race condition here where the owner and permissions of + // the file can change in the time between we validate the owner and + // permissions and read the contents of the file. We deem the chance + // of that to be small enough to ignore. + log.debug('Reading the ipc connection info from', rpcAddressFile); fs.readFile(rpcAddressFile, 'utf8', function (err, data) { @@ -356,3 +367,15 @@ const appDelegate = { }; appDelegate.setup(); + +function isOwnedAndOnlyWritableByRoot(path) { + const stat = fs.statSync(path); + const isOwnedByRoot = stat.uid === 0; + + // Taken from gagle's comment at https://github.com/nodejs/node-v0.x-archive/issues/3045#issuecomment-4865547 + const modeAsOctalString = (stat.mode & parseInt('777', 8)).toString(8); + const isOnlyWritableByOwner = modeAsOctalString === '604'; + + log.debug(path, 'is owned by', stat.uid, 'and has permsissions', modeAsOctalString); + return isOwnedByRoot && isOnlyWritableByOwner; +} |
