Skip to content

Commit

Permalink
Fix a vulnerability from a crafted argument to 'bunyan -p ARG'
Browse files Browse the repository at this point in the history
This was reported privately as:
    https://hackerone.com/reports/902739
    bunyan - RCE via insecure command formatting

After this change:
    % ./bin/bunyan -p "S'11;touch hacked ;'\\"
    bunyan: error: no matching PIDs found for "S'11;touch hacked ;'\"

With _DEBUG self-logging to show the escaped command:
    % ./bin/bunyan -p "S'11;touch hacked ;'\\"
    (bunyan: exec cmd: "ps -A -o pid,command | grep '[S]'\\''11;touch hacked ;'\\''\\\\'")
    bunyan: error: no matching PIDs found for "S'11;touch hacked ;'\"
    (bunyan: cleanupAndExit)
    (bunyan: process.exit(2))

Before this change these would create a "hacked" file in the current dir.
  • Loading branch information
trentm committed Jun 24, 2020
1 parent 033b37d commit ea21d75
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 13 deletions.
12 changes: 12 additions & 0 deletions CHANGES.md
Expand Up @@ -10,6 +10,18 @@ Known issues:
(nothing yet)


## 1.8.13

- Fix a vulnerability from a crafted argument to 'bunyan -p ARG'

This was reported privately as:
https://hackerone.com/reports/902739
bunyan - RCE via insecure command formatting

Previous to this version the 'bunyan' CLI was not escaping a given argument
to the '-p' option before executing `ps -A -o pid,command | grep '$ARG'`
which could lead to unintended execution.

## 1.8.12

- [issue #444] Fix the `bunyan` CLI to not duplicate the "HTTP/1.1 ..." status
Expand Down
16 changes: 12 additions & 4 deletions bin/bunyan
@@ -1,7 +1,7 @@
#!/usr/bin/env node
/**
* Copyright 2017 Trent Mick
* Copyright 2017 Joyent Inc.
* Copyright 2020 Trent Mick
* Copyright 2020 Joyent Inc.
*
* bunyan -- filter and pretty-print Bunyan log files (line-delimited JSON)
*
Expand All @@ -11,7 +11,7 @@
* vim: expandtab:ts=4:sw=4
*/

var VERSION = '1.8.12';
var VERSION = '1.8.13';

var p = console.log;
var util = require('util');
Expand Down Expand Up @@ -1266,7 +1266,15 @@ function processPids(opts, stylize, callback) {
// own search.
regex = '[' + regex[0] + ']' + regex.slice(1);
}
exec(format('ps -A -o pid,command | grep \'%s\'', regex),
var cmd = format('ps -A -o pid,command | grep \'%s\'',
// Escape single-quotes to avoid breaking the grep arg quoting
// (leading to a possible *code execution*) and backslashes to
// avoid undoing that escaping.
regex.replace(/\\/g, '\\\\')
// JSSTYLED
.replace(/'/g, "'\\''"));
if (_DEBUG) { warn('(bunyan: exec cmd: %j)', cmd); }
exec(cmd,
function (pidsErr, stdout, stderr) {
if (pidsErr) {
warn('bunyan: error getting PIDs for "%s": %s\n%s\n%s',
Expand Down
2 changes: 1 addition & 1 deletion lib/bunyan.js
Expand Up @@ -8,7 +8,7 @@
* vim: expandtab:ts=4:sw=4
*/

var VERSION = '1.8.12';
var VERSION = '1.8.13';

/*
* Bunyan log format version. This becomes the 'v' field on all log records.
Expand Down
20 changes: 12 additions & 8 deletions package.json
@@ -1,21 +1,27 @@
{
"name": "bunyan",
"version": "1.8.12",
"version": "1.8.13",
"description": "a JSON logging library for node.js services",
"author": "Trent Mick <trentm@gmail.com> (http://trentm.com)",
"main": "./lib/bunyan.js",
"bin": {
"bunyan": "./bin/bunyan"
},

"repository": {
"type": "git",
"url": "git://github.com/trentm/node-bunyan.git"
},
"engines": ["node >=0.10.0"],
"keywords": ["log", "logging", "log4j", "json", "bunyan"],
"engines": [
"node >=0.10.0"
],
"keywords": [
"log",
"logging",
"log4j",
"json",
"bunyan"
],
"license": "MIT",

"// dtrace-provider": "required for dtrace features",
"// mv": "required for RotatingFileStream",
"// moment": "required for local time with CLI",
Expand All @@ -32,10 +38,8 @@
"verror": "1.3.3",
"vasync": "1.4.3"
},

"scripts": {
"test": "make test"
},
"dependencies": {
}
"dependencies": {}
}

0 comments on commit ea21d75

Please sign in to comment.