search

yyoon / Journaley

A simple and elegant open-source journal keeping software for Windows
http://journaleyapp.com/
Other
115 stars 19 forks source link

Journal entries are vulnerable to XSS attacks #118

Open Mobius3-7 opened 8 years ago

Mobius3-7 commented 8 years ago

Create a journal entry with the following as its content:

<script>alert("This could be an XSS attack!");</script>

You will see a dialog box pop up every time the journal entry is opened. This is a very serious security vulnerability as it allows arbitrary code execution in your application. You should fix it by sanitizing journal entries as soon as possible.

Mobius3-7 commented 8 years ago

In fact, Journaley seems to allow any sort of HTML tag. Putting the following in a journal entry will create a browser window inside of the entry. I suspect that isn't intended behavior.

 <iframe src="http://www.w3schools.com"></iframe>
yyoon commented 8 years ago

Yes, Journaley allows any HTML tag and JavaScript code in an entry.

Note that the situation is very different from normal websites with XSS vulnerability. You're only reading the journal entries that you've written, not by others. That is why the possibility of XSS attack was not taken very seriously in the first place.

Do you have any specific attack scenario using Journaley in your mind?

Mobius3-7 commented 8 years ago

I agree that Journaley isn't likely to be a target for an XSS attack. Most likely, someone would have to either trick a user into entering a script or compromise a journal file some other way. You don't have any control over situations like that.

Even so, you are providing your users with the means to execute arbitrary JavaScript and view arbitrary web pages inside your app. I would like to argue that those things shouldn't be within the scope of your application. I have virtually no JavaScript experience, but I was able to do was verify that you can use JQuery and can't use the HTML 5 file API. Does your typical user need JQuery to write their journal entries?

On principle alone, I think you should sanitize journal entries to remove scripts and other potentially dangerous HTML entities.

sguergachi commented 8 years ago

Unless JavaScript is required for rendering entires and depending on difficulty (not sure how hard it would be to disable JavaScript in the IE webview) I see no reason we shouldn't disable it as a precaution. I think what @yyoon is suggesting is that this might not be as high priority as it sounded in the initial post.

yyoon commented 8 years ago

Yes. I totally agree that sanitizing is a good idea in principle. Just wanted to know if there's a serious immediate threat, because then we would have to resolve this issue right away.

Disabling JavaScript sounds like a good idea, too.

Mobius3-7 commented 8 years ago

I understand what you are saying now. The only ways that I can think of to exploit this involve another, more important, part of the system being compromised already.

jottinger commented 8 years ago

Maybe look at NSoup (http://www.developerfusion.com/project/98472/nsoup/) or dcsoup (https://github.com/matarillo/dcsoup) as cleaners for script? I don't think arbitrary HTML is a bad thing (although I'd prefer Markdown or AsciiDoc as options, honestly - with AsciiDoc being far preferred between them) but HTML is more useful than plain text.

yyoon commented 8 years ago

@jottinger Thanks for the pointers. I will take a look at those two and decide what to do. FYI, Markdown is already supported in Journaley if you didn't know about it. That is why we're using a browser control to display the journal contents in the first place.

jottinger commented 8 years ago

@yyoon , I noticed that after looking at the resources in the source tree - tried it, and it works. I don't like how it's rendered in Markdown after it's saved, but that's such a minor annoyance that it's not really worth considering. Excellent program, much appreciated. Considering trying to see if I can build a mac assembly for it. :)

sguergachi commented 8 years ago

Day One is an excellent Mac alternative:)

On Nov 9, 2015, 11:21 PM -0500, Joseph Ottingernotifications@github.com, wrote:

@yyoon(https://github.com/yyoon), I noticed that after looking at the resources in the source tree - tried it, and it works. I don't like how it's rendered in Markdown after it's saved, but that's such a minor annoyance that it's not really worth considering. Excellent program, much appreciated. Considering trying to see if I can build a mac assembly for it. :)

— Reply to this email directly orview it on GitHub(https://github.com/yyoon/Journaley/issues/118#issuecomment-155282077).

jottinger commented 8 years ago

@yyoon, with Dropbox integration, the script thing could actually be used as an exploit, using dropbox as an attack vector. Consider a shared directory (or a hacked dropbox account): I could write a journal entry that exploited the host system, and Journaley could (potentially) execute the code... I don't know what attacks are possible (not familiar enough with the .net rendering environment) but I can see the attack vector potential.

jottinger commented 8 years ago

@sguergachi yeah, I have Day One for the Mac - Journaley's ability to use Day One's data is awesome, because I work on multiple platforms. What I'd like to use is the same platform on all of them. :)