This commit upgrades dependencies that are more or less trivial
to update, e.g. because they didn't have major version bumps or
simply didn't break anything. There are some dependencies which
have not been upgraded since this would have required larger
refactorings. This includes especially the markdown-it ecosystem
and the webpack ecosystem.
The largest refactorings in this commit come from the bump of
socket.io v2 to v4 which changed the handling of the connected
socket list for instance.
This commit further removes some outdated and/or unnecessary
dependencies. This includes the String.js library which is
unmaintained for 9 years and has some CVEs. We mainly used
this library for their escapeHTML and unescapeHTML methods.
This can be done using native DOM APIs nowadays, which is also
considered more safe since it is the same logic that the
browser itself uses.
Since we target Node 18 and above, we can also rely on the
built-in fetch function instead of the node-fetch package.
The current version of Chance.js includes a method for
generating a random color now too, so we don't need the
package randomcolor anymore.
Signed-off-by: Erik Michelson <github@erik.michelson.eu>
shortid is deprecated and they recommend nanoid instead.
We're not sure if this has to do with possible name
collisions or enumerability, but to be sure and on the
safe side, we're changing this. nanoid seems quite safe
since it uses node's crypto module underneath.
Signed-off-by: Erik Michelson <github@erik.michelson.eu>
MySQLs string comparisons are case-insensitive by default.
This allows to hide notes by creating a new note with an alias that
equals the lower-cased alias of another note.
The new note is returned first by MySQL, so the original one is not
accessible anymore.
This fixes the problem by using an explicit binary comparison in the
affected queries.
See https://dev.mysql.com/doc/refman/8.0/en/case-sensitivity.html
Signed-off-by: David Mehren <git@herrmehren.de>
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>
As explained in #391, the previous note creation logic didn't handle the case "alias is set, but it's not a file on disk". The fix introduces a separate if-statement for this scenario at the cost of a doubled filesystem read access.
Co-Authored-By: @evanlinde
Signed-off-by: Erik Michelson <github@erik.michelson.eu>
This does some more in depth check on the error message and minimizes
the log noise that is caused by LZString.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
When we delete a user we should delete all the notes that belong to this
user including the revisions of these notes.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
Right now we only flag notes as deleted. This is no longer allowed under
GDPR. Make sure you do regular backups!
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
As we know the length of an UUID we can check if the base64 string
of the provided UUID is long enough for a legacy base64 encoded nodeId
and stop processing it in legacy mode, if it's not the case.
This should make the ugly warning way less common.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
This refactors the configs a bit to now use camel case everywhere.
This change should help to clean up the config interface and make it
better understandable.
Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>
