diff options
author | Sheogorath | 2021-03-23 00:02:30 +0100 |
---|---|---|
committer | Sheogorath | 2021-04-25 20:40:17 +0200 |
commit | 44b7f607a542abc2f47ac141f2fd6cd1d34ed1c5 (patch) | |
tree | 30013f2d8085ce04c004c6389725c4beb9c2cb36 | |
parent | 2ea40bb98d80ca765f60f9b69d26d5be12188231 (diff) |
Fix Relative Path Traversal Attack on note creation
Impact
---
An attacker can read arbitrary `.md` files from the server's filesystem due to an [improper input validation](https://cwe.mitre.org/data/definitions/20.html), which results in the ability to perform a [relative path traversal](https://cwe.mitre.org/data/definitions/23.html).
CVSSv3 string: AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N
PoC / Quicktest
---
To verify if you are affected, you can try to open the following URL: `http://localhost:3000/..%2F..%2FREADME#` (replace `http://localhost:3000` with your instance's base-URL e.g. `https://demo.hedgedoc.org/..%2F..%2FREADME#`).
- If you see a README page being rendered, you run an affected version.
Analysis
---
The attack works due the fact that [the internal router, passes the url-encoded alias](https://github.com/hedgedoc/hedgedoc/blob/master/lib/web/note/router.js#L26) to the `noteController.showNote`-function. This function passes the input directly to [`findNote()`](https://github.com/hedgedoc/hedgedoc/blob/78a732abe691b496fa3692aa2add37f7344db1fa/lib/web/note/util.js#L10) utility function, that will pass it on the the [`parseNoteId()`](https://github.com/hedgedoc/hedgedoc/blob/78a732abe691b496fa3692aa2add37f7344db1fa/lib/models/note.js#L188-L258)-function, that tries to make sense out of the noteId/alias and check if a note already exists and if so, if a corresponding file on disk was updated.
If no note exists the [note creation-function is called](https://github.com/hedgedoc/hedgedoc/blob/78a732abe691b496fa3692aa2add37f7344db1fa/lib/models/note.js#L240-L245), which pass this unvalidated alias, with a `.md` appended, into a [`path.join()`-function](https://github.com/hedgedoc/hedgedoc/blob/78a732abe691b496fa3692aa2add37f7344db1fa/lib/models/note.js#L99) which is read from the filesystem in the follow up routine and provides the pre-filled content of the new note.
This allows an attacker to not only read arbitrary `.md` files from the filesystem, but also observes changes to them.
The usefulness of this attack can be considered limited, since mainly markdown files are use the file-ending `.md` and all markdown files contained in the hedgedoc project, like the README, are public anyway. If other protections such as a chroot or container or proper file permissions are in place, this attack's usefulness is rather limited.
Workarounds
---
On a reverse-proxy level one can force a URL-decode, which will prevent this attack because the router will not accept such a path.
For more information
---
If you have any questions or comments about this advisory:
* Open an topic on [our community forum](https://community.hedgedoc.org)
* Join our [matrix room](https://chat.hedgedoc.org)
Advisory link
---
https://github.com/hedgedoc/hedgedoc/security/advisories/GHSA-p528-555r-pf87
Signed-off-by: Christoph (Sheogorath) Kern <sheogorath@shivering-isles.com>
-rw-r--r-- | lib/models/note.js | 6 |
1 files changed, 3 insertions, 3 deletions
diff --git a/lib/models/note.js b/lib/models/note.js index 7b8b6783..0c268e2e 100644 --- a/lib/models/note.js +++ b/lib/models/note.js @@ -94,7 +94,7 @@ module.exports = function (sequelize, DataTypes) { let body = null let filePath = null if (note.alias) { - filePath = path.join(config.docsPath, note.alias + '.md') + filePath = path.join(config.docsPath, path.basename(note.alias) + '.md') } if (!filePath || !Note.checkFileExist(filePath)) { filePath = config.defaultNotePath @@ -196,7 +196,7 @@ module.exports = function (sequelize, DataTypes) { } }).then(function (note) { if (note) { - const filePath = path.join(config.docsPath, noteId + '.md') + const filePath = path.join(config.docsPath, path.basename(noteId) + '.md') if (Note.checkFileExist(filePath)) { // if doc in filesystem have newer modified time than last change time // then will update the doc in db @@ -238,7 +238,7 @@ module.exports = function (sequelize, DataTypes) { return callback(null, note.id) } } else { - const filePath = path.join(config.docsPath, noteId + '.md') + const filePath = path.join(config.docsPath, path.basename(noteId) + '.md') if (Note.checkFileExist(filePath)) { Note.create({ alias: noteId, |