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 CVE-2022-24434 #179

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Fix CVE-2022-24434 #179

merged 1 commit into from
Aug 24, 2022

Conversation

XeR
Copy link
Contributor

@XeR XeR commented Aug 23, 2022

I hand-patched the vulnerable dependency to a non-vulnerable version.

I did this by:

  1. removing dicer and its dependency from the yarn lockfile
  2. changing the dicer requirement to upgrade its version to 0.3.1
  3. running yarn add to add dicer@0.3.1
  4. remove the direct dependency on dicer from package.json

I invited you to a private repository with a PoC script to test the fix.
The container api crashes before the patch, now it drops the request

Force dicer to upgrade to version 0.3.1 by editing the package lock by hand.
This protects against CVE-2022-24434.
@XeR XeR requested a review from JJ-8 August 23, 2022 20:42
Copy link
Collaborator

@JJ-8 JJ-8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the patch fixes the problem.

We can also 'mitigate' this type of issue more generic:

diff --git a/docker-compose.yml b/docker-compose.yml
index 94cb353e..0d40b88e 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -5,6 +5,7 @@ services:
       context: "./api"
     networks:
       - ctfnote
+    restart: always
     environment:
       PAD_CREATE_URL: http://hedgedoc:3000/new
       PAD_SHOW_URL: /

If the server crashes, we just restart it always. Of course an attacker can launch the attack again and again, but this at least prevents the server from being down permanently until restarted by the admin.
This would also mitigate downtime due to errors in the code for example.

What do you think of this @XeR?

@JJ-8 JJ-8 merged commit 64a73bc into TFNS:dev Aug 24, 2022
@XeR XeR deleted the 0-CVE-2022-24434 branch August 24, 2022 18:45
@XeR
Copy link
Contributor Author

XeR commented Aug 24, 2022

What do you think of this @XeR?

👍
I let you submit the PR so you get credit

JJ-8 added a commit to JJ-8/CTFNote that referenced this pull request Aug 26, 2022
In case of DoS, the container will restart instead of it being down
until restarted manually.

As discussed in TFNS#179
XeR pushed a commit that referenced this pull request Aug 26, 2022
In case of DoS, the container will restart instead of it being down
until restarted manually.

As discussed in #179
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants