edbrowse-dev - development list for edbrowse
 help / color / mirror / Atom feed
From: Adam Thompson <arthompson1990@gmail.com>
To: Karl Dahlke <eklhad@comcast.net>
Cc: edbrowse-dev@edbrowse.org
Subject: Re: [edbrowse-dev] interwindow bleed
Date: Sun, 15 Nov 2020 12:26:57 +0000	[thread overview]
Message-ID: <20201115122657.GA10863@toaster> (raw)
In-Reply-To: <20201009202834.eklhad@comcast.net>

On Mon, Nov 09, 2020 at 08:28:34PM -0500, Karl Dahlke wrote:
> > surprised (and annoyed with my self) that I didn't think about this...
> 
> Yeah, me too. Ha ha.
> It's a funny thing about security. If you're not thinking about it, you're really not thinking about it. Then when you do think about it, sometimes it only takes 5 minutes of thought to see the problems.
> Like turning a light on.
> It just came to me in the shower.

Indeed.  Given that I used to work for a cybersecurity company (all be it on the
web proxy component) I should've been more alert to this I think.

> > Does that mean it won't be a "master window" without that built?
> 
> As of 3.7.7, and in production, it's just an empty object, unless you build the way I do,
> with EBDEMIN=1, then all our tracing and debugging and deminimization stuff goes in there.

sorry if this reflects a lack of knowledge but does the presence of this
global object still open an, admittedly potentially contrived and
Edbrowse-specific, path to break the new isolation?

> > I hope it wasn't that bad.
> 
> Oh I didn't mean it in a bad way. I have chronic insomnia, or more accurately, non24,
> so if I have something interesting to work on at night, that's good.
> It was a pretty easy change, actually, to build all our stuff in each window, rather than once in the master window.

I'm glad that it wasn't too difficult to fix.

> It didn't take long to do that, and now windows are properly isolated, well, except for frames, and that's as it should be.
> I've seen more and more sites use code like this:
> 
> if(top != window) { stop and don't do anything }
> 
> This guards against a security attack where your juicy site is pulled in as a frame of a larger site,
> maybe the frame is your bank, and you log in, and the larger site that owns this frame can dip in and see your login and password.
> Well, top is the top window that has all the frames,
> and not the current window, which is your bank,
> so that simple test comparing top and window guards against interwindow bleed
> through the frame system.

One of my friends has just rewritten a bunch of html docs which did creative
things with frames.  This is apparently because some browsers are now
isolating frames (i.e.  the above guard) without requiring sites to do this.
I've no other reference for this currently but it was a rather substantial
rewrite and thus I suspect it was indeed required.

> Even if we can stay with duktape, which would be nice, I'm learning alot by this project, c++ for example, now have a better feel for it,
> and the nuts and bolts of the spider monkey api.
> I've already confirmed it is up to date, and handles all the things that duktape doesn't.
> And it led me to the security hole, so it's all good.

Indeed.  Often it pays to revisit code like this.

> It would be really great if we could build either or both at will in the ongoing future,
> but I don't think we can.
> The changes would spread outside of jseng_whatever.c and into three or four other C, perhaps having to become c++, files.
> I'm losing some of my encapsulation.

Indeed.  As I said it's a real shame if duktape isn't keeping up.

As a question, what is the promise stuff they're lacking? It looked like
they implemented this but I really don't know js enough to have an opinion.
I also wonder whether we need to embed duktape (which I know I've previously
spoken against) so that we can potentially enable anything which is switched
off in the default build.  It looks like, if this genuinely buys us any
features, it'd just be a .c and .h file to pull in.

Just food for thought.

Cheers,
Adam.


  reply	other threads:[~2020-11-15 12:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-06  4:22 Karl Dahlke
2020-11-09 23:11 ` Adam Thompson
2020-11-10  1:28   ` Karl Dahlke
2020-11-15 12:26     ` Adam Thompson [this message]
2020-11-15 12:45       ` Karl Dahlke
2020-11-16  3:27       ` Kevin Carhart
2020-11-16  7:54         ` Adam Thompson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201115122657.GA10863@toaster \
    --to=arthompson1990@gmail.com \
    --cc=edbrowse-dev@edbrowse.org \
    --cc=eklhad@comcast.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).