csaf-poc / csaf_webview

Web app (module) to display a CSAF 2 document and to browse CSAF 2 ROLIE feeds. ⚠️ The web demo is often not allowed to access servers:
https://csaf-poc.github.io/csaf_webview/
1 stars 3 forks source link

Support for Markdown #32

Closed ThomasJunk closed 10 months ago

ThomasJunk commented 10 months ago

Commit f968cbee9136df03df0919ddcae2ad9373baef58 includes now the support for (github flavoured) Markdown. To test you could modify e.g. the notes of a document like

"notes": [
        {
          "category": "description",
          "text": "**CSAF Tools CVRF-CSAF-Converter 1.0.0-rc1** resolves XML External Entities (XXE). _This_ leads to the inclusion of arbitrary (local) file content into the generated output document. An attacker can exploit this to disclose information from the system running the converter.",
          "title": "Vulnerability description"
        }
      ],

It should render the text properly.

Basically any text could now contain Markdown which would be rendered.

This issue is open to test the behaviour.

s-l-teichmann commented 10 months ago

I have added this:

"notes" : [ {
      "category" : "description",
      "text" : "... <code onclick=\"alert('I was here!');\">Click me</code>"
    } ],

Results in: alert

I don't think the HTML vetting is tough enough.

s-l-teichmann commented 10 months ago

https://docs.oasis-open.org/csaf/csaf/v2.0/os/csaf-v2.0-os.html#8-safety-security-and-data-protection-considerations

ThomasJunk commented 10 months ago

hmm... bummer...

ThomasJunk commented 10 months ago

I removed Svelte-Markdown from the stack in fc2139a0c5e3f3e00a678ddc4eb5ccb9055b5dac Seems the best solution for now.

ThomasJunk commented 10 months ago

On the development branch there is now another attempt. Commit https://github.com/csaf-poc/csaf_webview/commit/fd61465eea688a652560730e55b280f61ea9714b now uses marked.js and DOMPurify to produce a sanitized output.

This should be better.

s-l-teichmann commented 10 months ago

The code eval is gone ... as it ist the markdown handling :-/

no-effect

Edit: Ah! On the development branch is the new stuff.

s-l-teichmann commented 10 months ago

development branch looks fine. no-event

The inspector shows that there is no event tag generated which is fine. :-)

s-l-teichmann commented 10 months ago

The question reamains how 'Github-flavoured' this markdown is.

ThomasJunk commented 10 months ago

The question reamains how 'Github-flavoured' this markdown is.

Somewhat "decent"... from what I found: https://github.com/markedjs/marked/discussions/1202#discussioncomment-4192078

ThomasJunk commented 10 months ago

With commit https://github.com/csaf-poc/csaf_webview/commit/98164a364544cac617222f37ae0ff5f0d912fee9 I added a visual clue that the content rendered is actually markdown.

markdown_hint

bernhardreiter commented 10 months ago

It seems text fields are marked "markdown" even if they are just text without any markup. Like the descriptions of the CVEs in in https://wid.cert-bund.de/.well-known/csaf/white/2023/wid-sec-w-2023-2995.json . How do we determine which fields are markdown and which aren't?

The marking is a good start, can the gray box be placed to the lower right, inside the box? This way it is getting less in the way.

ThomasJunk commented 10 months ago

The marking is a good start, can the gray box be placed to the lower right, inside the box?

This should be possible.

How do we determine which fields are markdown and which aren't?

I think it's hard to determine whether or not a text field is plain text or actually markdown. You could have hyphens at the start of a line without it actually being "markdown" but of course can assume they're meant to be bullet points and render them as such.

ThomasJunk commented 10 months ago

markdown

Changed in https://github.com/csaf-poc/csaf_webview/commit/b363c2f4408d58aa9128e471e61d7fb6971ab248

ThomasJunk commented 10 months ago

In the meantime: May you test @JanHoefelmeyer ?

JanHoefelmeyer commented 10 months ago

Testing: Looks the same for me

bernhardreiter commented 10 months ago

It seems we cannot reliably detect markdown (https://stackoverflow.com/a/24690466 hints how to calculate a likelyhood), so my suggestion is: we keep trying to render text and use a box. But we shall remove the "markdown" gray box, as it can be missleading.

ThomasJunk commented 10 months ago

textbox shows the box but not the markdown hint.

ThomasJunk commented 10 months ago

In order to test this issue one has to check out the ui-refactoring branch with latest commits.

JanHoefelmeyer commented 10 months ago

Test successful, works as described in https://github.com/csaf-poc/csaf_webview/issues/32#issuecomment-1829523788