mirror of
https://github.com/hedgedoc/hedgedoc.git
synced 2024-11-25 03:06:31 -05:00
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()`](78a732abe6/lib/web/note/util.js (L10)
) utility function, that will pass it on the the [`parseNoteId()`](78a732abe6/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](78a732abe6/lib/models/note.js (L240-L245)
), which pass this unvalidated alias, with a `.md` appended, into a [`path.join()`-function](78a732abe6/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>
This commit is contained in:
parent
2ea40bb98d
commit
44b7f607a5
1 changed files with 3 additions and 3 deletions
|
@ -94,7 +94,7 @@ module.exports = function (sequelize, DataTypes) {
|
||||||
let body = null
|
let body = null
|
||||||
let filePath = null
|
let filePath = null
|
||||||
if (note.alias) {
|
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)) {
|
if (!filePath || !Note.checkFileExist(filePath)) {
|
||||||
filePath = config.defaultNotePath
|
filePath = config.defaultNotePath
|
||||||
|
@ -196,7 +196,7 @@ module.exports = function (sequelize, DataTypes) {
|
||||||
}
|
}
|
||||||
}).then(function (note) {
|
}).then(function (note) {
|
||||||
if (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 (Note.checkFileExist(filePath)) {
|
||||||
// if doc in filesystem have newer modified time than last change time
|
// if doc in filesystem have newer modified time than last change time
|
||||||
// then will update the doc in db
|
// then will update the doc in db
|
||||||
|
@ -238,7 +238,7 @@ module.exports = function (sequelize, DataTypes) {
|
||||||
return callback(null, note.id)
|
return callback(null, note.id)
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
const filePath = path.join(config.docsPath, noteId + '.md')
|
const filePath = path.join(config.docsPath, path.basename(noteId) + '.md')
|
||||||
if (Note.checkFileExist(filePath)) {
|
if (Note.checkFileExist(filePath)) {
|
||||||
Note.create({
|
Note.create({
|
||||||
alias: noteId,
|
alias: noteId,
|
||||||
|
|
Loading…
Reference in a new issue