Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Remote Code Execution vulnerability on plugins install and update #5286

wants to merge 1 commit into from


Copy link

commented Dec 14, 2016

No description provided.


This comment has been minimized.

Copy link

commented Dec 14, 2016

CLA assistant check
All committers have signed the CLA.

jarey approved these changes Dec 14, 2016

This comment has been minimized.

Copy link

commented Dec 14, 2016

A malicious user could be able to run shell commands by forcing admins to run special Javascript code like: socket.emit("admin.plugins.upgrade",{"id":"nodebb-plugin","version":"0.1.2; ls -l"})

There are multiple XSS vulnerabilities to achive that.

@julianlam julianlam closed this in e028ac1 Dec 14, 2016


This comment has been minimized.

Copy link

commented Dec 14, 2016

Thanks @segura2010 -- I ended up fixing this issue differently. Instead of santizing the command, we now use execFile instead of exec. Good catch!


@julianlam julianlam added this to the 1.4.1 milestone Dec 14, 2016


This comment has been minimized.

Copy link

commented Dec 14, 2016

If you could try to exploit it again, that would be good. Otherwise we'll release 1.4.1 by end of day, likely.


This comment has been minimized.

Copy link

commented Dec 14, 2016

I just checked and it is fixed :)

meulta added a commit to meulta/NodeBB that referenced this pull request Jan 5, 2017
Pull from master (#1)
* NodeBB#5223

adjust pagination so each page shows `postsPerPage` posts

* tag controller test

* Latest translations and fallbacks

* trimming composer input before doing length check in replies

* NodeBB#5223

* fixes NodeBB#5225

* fix topic creation regression caused by 576df84

* maintenance tests

* fix test

* move maintenance mode

* suggested topics test

* use global mod user for flag tests

* some more group tests

* more group tests

* account group page test

* fixes NodeBB#5228

* resolve regression caused by part of d28f7de

* fixing new post parsing to not add img-responsive to avatars

* all procs should load the js from file

* removed file.exists from getFromFile

* categories update test

* vote socket tests

* dont use hardcoded uid

* digest test

* closes NodeBB#5230

* Latest translations and fallbacks

* multiple test fixes

* allow two sections

* fixes NodeBB#5226

* Standard language codes (NodeBB#5218)

* Use standard language codes. Fallback for plugins.

* Fix transifex config

* Tab vs space here for some reason

* Remove redundancies

* config.relative_path instead of allcaps

* added upgrade script for existing users' accounts

* fixes NodeBB#5230

* linting

* remove async.series

* fix headers for new installs

encodeURI(undefined) === "undefined"

* closes NodeBB#5231

* use base_dir

* closes NodeBB#5234

* up persona

* removed incorrect markread nid

* switch to createIndex

* up dbsearch

* up markdown and composer versions

* some tweaks to the readme

* up markdown

* change pagination so its similar to GH

* fix test dont turn single pages into ...

* store pinned topics in new zset

keep pinned topics on top on different sort types

* dont mark chat notifications read if you are not in the room

* closes NodeBB#5236

* fix permalinks on pagination

* more group tests

* fix update privacy

* check deps on test

* closes NodeBB#5238

* topics unread tests

* closes NodeBB#5239

* ACP search updated to support translations

* Explanations and simplifications

* Tests for admin search, simplifications

* Use config.relative_path instead of allcaps

* Remove unnecesary admin search indexing

* Fixes, passes tests

* closes NodeBB#5241

* don't crash if plugin doesnt set defaultLang

* show chat room title in taskbar

* removed comment from Gruntfile

* closes NodeBB#5242

* test plugin static assets

* account/posts controller tests

* remove placeholder NodeBB#5242

* closes NodeBB#5245

* build js in parallel

* group search tests

* fix style

* follow tests

* more socket user tests

* remove hardcoded value @pichalite

* priv table headers

* v1.4.0

* upped composer version

* more tests

group cover upload tests
registration approval queue tests

* search socket test

* more tests

* more tests

* locking down session deletion route to admins and global mods only

* linting :shipit:

* category tests

* move post test

* updated revoke session middleware to allow self or admin or global mod invocation, tweaked tests a bit

* ZSET scores are float: parseInt => parseFloat

In Redis, scores of sorted sets can be floats – so we should use `parseFloat` instead of `parseInt` when converting from string to number.
Should not lead to NodeBB#4939 again, as `new Date()` works regardless of whether it's being passed a float or integer.

* tag tests

* fix post move test

* notif controller test

* closes NodeBB#5254

* tweak gitignores, closes NodeBB#5250

* closes NodeBB#5251

* Use async instead of Promises

* Fix Translator to work with namespace paths

* Translate skins and themes fully

* Key through search results

* closes NodeBB#5249

* closes NodeBB#5259

* Avoid encoding HTML in Twitter social share text

* Fix linting error

* closes NodeBB#5262

* convert topic title to string

* up'd tjs

* fixes NodeBB#5263

* closes NodeBB#5271

* fix lint

* parallel startup

* show seconds in logs

* closes NodeBB#5273

* up composer

* change group membership methods

* meta/settings test

* Escape and ignore `%` and `\,` in translator args

* Restructure and rename translator tests

* Add tests for translator static methods

* Escape arguments in `Translator.compile`

* Fixes for dev-ing on windows

- Change `nodebb.bat` to simply run `node ./nodebb` with same arguments
- Fix `npm test` for windows

* mongo: set scores as float instead of int

* sneakily adjust database/sorted tests to include float scores

* Check if href exists before accessing it (NodeBB#5281)

I got a lot of errors in Firefox 50 `TypeError: $(...).attr(...) is undefined  nodebb.min.js:25167:24` which points exactly to that line I’ve changed.
Since HTML5 `href` is not a required attribute of an `a` tag. We have a couple of links without `href` and every time you click it you will get this error.

* Check password length on setup

* Fix tabs

* closes NodeBB#5276

* add cid to widgets.render

init date pickers in widgets ACP

* filter:middleware.render

* fix hook

* use build module instead of forking

* missing ;

* request travis to build  now too

* new hook, action:user.delete

* fix ajax 404 err when base url isn't root (NodeBB#5285)

* passing in arguments to npm instead of command string, closes NodeBB#5286

* closes NodeBB#5287

* closes NodeBB#4563 closes NodeBB#4569 closes NodeBB#4566

* closes NodeBB#4544

* notification tests

* tag tests

* more tag tests

* meta.configs tests

* closes NodeBB#5290

* what, does Travis not like 4-space indents?

* escape labelColor, icon, cover:position, validate toPid

* up composer

* up mongodb

* ability to filter search by tags

* moved next() of out try/catch

* up themes

* Incremented version number

* `admin/advanced` translations

* `admin/appearance` translations

* `admin/development` translations

* `admin/extend` translations

* Translate dynamically added admin content

* `admin/general` translations

* Translation bootbox wrapper

- Replaced minfied bootbox file with unminified one since it's minified at build anyways
- Removed existing override
- Made translator more verbose in dev mode; it now warns about missing translations

* `admin/advanced` JS translations

* Bootbox wrapper improvements

* ACP menu and title translations

* `admin/extend` JS translations and misc

* `admin/general` JS translations and misc

* ACP search and title improvements

- Search uses translated titles if available
- Use `advanced` for `development` route titles
- Remove route title from showing up in results
- Highlight matching part of result title
- Don't show empty result contents when only title is matched

* `admin/manage/categories` translations

- Fix privilege table headers so bottom borders align
- Fix `/admin` route to show Dashboard title correctly
- Translate ACP category management and privileges templates
- Translate ACP category management JS
- Remove unnecessary translates in JS
- Fix bootbox wrapper to work with translations containing html

* `admin/manage` translations, misc

- Translate Manage templates and JS
- Change `translator.translate -> .html` into `.translateHtml` where fitting
- Translate `admin/partials/download_plugin_item`


* Fix ajaxify loading default language translations

* ACP Translations

* rooms.getAll test

* fix tests

* fix missing semicolon

* post tools test

* up persona

* allowing timeago timestamps to be in the future

* up persona: new quick reply option

* add missing return, fix tests

* Add missing translation

* up persona

* closes NodeBB#5300

* no need to check for logged in status in ACP

* fixes `npm test` on windows

* dont add tid to :tids:posts if its pinned

* closes NodeBB#3973 closes NodeBB#5303

* Fix ACP title issues

* updating to 2017 (was looking for a reason to test auto-update of

* up composer

* prevent crash if topic is not found

* use different path for installer files

* Incremented version number

* closes NodeBB#5307

* Update ACE editor to latest

* Update ACE editor to latest

* organize dbal sorted code

* fix redis union

* refactor app.js/start

* fix requires

* move bookmarks

* remove old upgrade code

* fix dupe code

* fix missing ;

* refactor post tools

* fix tests

* up themes

* use dot instead of spec for tests

* remove double-click to mark all notifications read

* closes NodeBB#5314

* fix minSchemaDate in upgrade.js

* Let global mods change user avatar

* Confirm before removing user and group cover picture

* Delete cover position data when cover photo is deleted

* Use scrollStop in chat (NodeBB#5326)

* Add missing translations

* Add more user tests

* Add more user tests

* Fix ACP title bug with hashes (NodeBB#5331)

* closes NodeBB#5333

* messaging refactor

* added logo to .gitignore so it doesn't get updated by test runner

* Fix selection

Tweak to allow the shift + click behaviour for the checkboxes while still letting the browser behaviour work

* fix indents

* Update Chart.js dependency version

Update Chart.js dependency to fix chart issue on category analytics page.

* fix typo in language file

* fixed grammar in admin-manage-categories string

* Latest translations and fallbacks

* update arabic language.json

* Latest translations and fallbacks

* fix language codes, @pichalite

* up persona

* allow multiple tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
4 participants
You can’t perform that action at this time.