edbrowse-dev - development list for edbrowse
 help / color / mirror / Atom feed
* [edbrowse-dev] interwindow bleed
@ 2020-11-06  4:22 Karl Dahlke
  2020-11-09 23:11 ` Adam Thompson
  0 siblings, 1 reply; 7+ messages in thread
From: Karl Dahlke @ 2020-11-06  4:22 UTC (permalink / raw)
  To: edbrowse-dev

[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]

Guys - really?
We've had this architecture for over 2 years and none of you called me on it?
None of you said,

"Ok Karl, it's more efficient, but there's a security hole big enough to drive a truck through!"

We compile our dom in the master window, mw0, just once, not every window every time, saves time, saves memory, saves resources, ok fine, but,
then stop for 5 minutes and think what someone can do.
Write your own appendChild function that calls mine, by making a copy of mw0.appendChild, then put yours in its place.
Since yours does what mine does, no one will ever know.
But you intercept and you have access to "this".
Climb up to document, then spider and read the entire web page.
document.location gives the url he is looking at.
If it's juicy, gather up information and submit it by form.submit or xhr or whatever.

Dude - seriously.
Any sort of interwindow bleed is a security hole.

It came to mind because sm doesn't share pages as easily as duktape.
I think I figured out how to do it, how to share the master window among all other windows,
but then I thought: I shouldn't be doing that anyways.

Ok so building everything in each window is a waste.
Adam said 8 years ago to write it all in C, and he's sort of right for this reason,
but rewriting js in c multiplies the code by about 10, no kidding,
4500 lines of js becomes 45,000 lines of c,
all of edbrowse so far after 20 years is about 45,000 lines,
so we clearly don't have the time or resources for that.
So we have to settle for where it is.
But in the grand scheme it's not so bad.
Web pages pull in jquery, which is ten times as big as my startwindow.js, after minimization, maybe 30 times before,
and that gets sucked in and processed on every page of the website, and if we're going to handle that,
then I guess we can build my dom each time too.

So speak now or forever, but I'm going to restructure it so it builds insitu each time,
(I like that word, insitu),
and then the master window, which I'll keep, is only for our third party deminimization software.
That doesn't even ship with production anyways.

So that will keep me up tonight.

Karl Dahlke

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edbrowse-dev] interwindow bleed
  2020-11-06  4:22 [edbrowse-dev] interwindow bleed Karl Dahlke
@ 2020-11-09 23:11 ` Adam Thompson
  2020-11-10  1:28   ` Karl Dahlke
  0 siblings, 1 reply; 7+ messages in thread
From: Adam Thompson @ 2020-11-09 23:11 UTC (permalink / raw)
  To: Karl Dahlke; +Cc: edbrowse-dev

On Thu, Nov 05, 2020 at 11:22:57PM -0500, Karl Dahlke wrote:
> Guys - really?
> We've had this architecture for over 2 years and none of you called me on it?
> None of you said,
> 
> "Ok Karl, it's more efficient, but there's a security hole big enough to drive a truck through!"
> 
> We compile our dom in the master window, mw0, just once, not every window every time, saves time, saves memory, saves resources, ok fine, but,
> then stop for 5 minutes and think what someone can do.
> Write your own appendChild function that calls mine, by making a copy of mw0.appendChild, then put yours in its place.
> Since yours does what mine does, no one will ever know.
> But you intercept and you have access to "this".
> Climb up to document, then spider and read the entire web page.
> document.location gives the url he is looking at.
> If it's juicy, gather up information and submit it by form.submit or xhr or whatever.
> 
> Dude - seriously.
> Any sort of interwindow bleed is a security hole.

Yeah...  surprised (and annoyed with my self) that I didn't think about
this...  I guess I just watched the commits land in the repo and didn't look
at the architecture they were creating... sorry about that.

> It came to mind because sm doesn't share pages as easily as duktape.
> I think I figured out how to do it, how to share the master window among all other windows,
> but then I thought: I shouldn't be doing that anyways.
> 
> Ok so building everything in each window is a waste.
> Adam said 8 years ago to write it all in C, and he's sort of right for this reason,
> but rewriting js in c multiplies the code by about 10, no kidding,
> 4500 lines of js becomes 45,000 lines of c,
> all of edbrowse so far after 20 years is about 45,000 lines,
> so we clearly don't have the time or resources for that.
> So we have to settle for where it is.
> But in the grand scheme it's not so bad.
> Web pages pull in jquery, which is ten times as big as my startwindow.js, after minimization, maybe 30 times before,
> and that gets sucked in and processed on every page of the website, and if we're going to handle that,
> then I guess we can build my dom each time too.

Agreed.  Honestly, performance's probably the least of our problems right
now.

> So speak now or forever, but I'm going to restructure it so it builds insitu each time,
> (I like that word, insitu),
> and then the master window, which I'll keep, is only for our third party deminimization software.
> That doesn't even ship with production anyways.

Does that mean it won't be a "master window" without that built? Or are you
going to still have it but ensure isolation in some other way? Or, if
present, will it merely be to hold the extra debugging functions etc?

> So that will keep me up tonight.

I hope it wasn't that bad.

Cheers,
Adam.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [edbrowse-dev] interwindow bleed
  2020-11-09 23:11 ` Adam Thompson
@ 2020-11-10  1:28   ` Karl Dahlke
  2020-11-15 12:26     ` Adam Thompson
  0 siblings, 1 reply; 7+ messages in thread
From: Karl Dahlke @ 2020-11-10  1:28 UTC (permalink / raw)
  To: edbrowse-dev

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

> 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.

> 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.

> 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.
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.

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.

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.

Karl Dahlke

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edbrowse-dev] interwindow bleed
  2020-11-10  1:28   ` Karl Dahlke
@ 2020-11-15 12:26     ` Adam Thompson
  2020-11-15 12:45       ` Karl Dahlke
  2020-11-16  3:27       ` Kevin Carhart
  0 siblings, 2 replies; 7+ messages in thread
From: Adam Thompson @ 2020-11-15 12:26 UTC (permalink / raw)
  To: Karl Dahlke; +Cc: edbrowse-dev

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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [edbrowse-dev] interwindow bleed
  2020-11-15 12:26     ` Adam Thompson
@ 2020-11-15 12:45       ` Karl Dahlke
  2020-11-16  3:27       ` Kevin Carhart
  1 sibling, 0 replies; 7+ messages in thread
From: Karl Dahlke @ 2020-11-15 12:45 UTC (permalink / raw)
  To: edbrowse-dev

> Does the presence of this global object still open a ... path ...

No. Here is an analogy.
You and I open a joint checking account, with nothing in it.
I can't steal any money from you unless you put it there.
A bank isn't going to put its valuable customer data in this shared object for others to look at.

Karl Dahlke


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edbrowse-dev] interwindow bleed
  2020-11-15 12:26     ` Adam Thompson
  2020-11-15 12:45       ` Karl Dahlke
@ 2020-11-16  3:27       ` Kevin Carhart
  2020-11-16  7:54         ` Adam Thompson
  1 sibling, 1 reply; 7+ messages in thread
From: Kevin Carhart @ 2020-11-16  3:27 UTC (permalink / raw)
  To: Adam Thompson, Karl Dahlke; +Cc: edbrowse-dev

Hi Adam

> 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.

It could be any and every ES6 feature.  Adoption is increasing as time goes by.  In July I worked on nasa.gov and got it working again.  Between July and now, they wrote in all kinds of ES6.  One example is a new use for the backtick symbol in place of quotation marks.  It's used in JS somewhat analogous to the use of backticks in bash etc.  So now nasa is broken again because of something you can't write a JS shim for.  (I've heard of transpiling to ES5, but I don't know how to do it or whether it's something we could do on the fly.)

Various sites are using 'let' a lot more.

We were working with dominos.com and they need postMessage.  We wrote a JS implementation of that one.

There's also the function bodies.  This one isn't ES6-related.  moz and v8 allow toString() of a function and return the source JS.  We think this is needed if we were to get ambitious and support paypal.com.  We spent a few weeks on it and solved several things but then ran into the buzzsaw of an obfuscated file they use which even has "traps".  And function bodies is part of what it's trying to detect.

It's too bad. duktape is terrific.  They don't not have the features, they're just someplace in the middle.  I think probably due to the massive amount of work it takes to incorporate them.

Kevin


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edbrowse-dev] interwindow bleed
  2020-11-16  3:27       ` Kevin Carhart
@ 2020-11-16  7:54         ` Adam Thompson
  0 siblings, 0 replies; 7+ messages in thread
From: Adam Thompson @ 2020-11-16  7:54 UTC (permalink / raw)
  To: Kevin Carhart; +Cc: Karl Dahlke, edbrowse-dev

On Sun, Nov 15, 2020 at 07:27:31PM -0800, Kevin Carhart wrote:
> Hi Adam
> 
> > 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.
> 
> It could be any and every ES6 feature.  Adoption is increasing as time goes by.  In July I worked on nasa.gov and got it working again.  Between July and now, they wrote in all kinds of ES6.  One example is a new use for the backtick symbol in place of quotation marks.  It's used in JS somewhat analogous to the use of backticks in bash etc.  So now nasa is broken again because of something you can't write a JS shim for.  (I've heard of transpiling to ES5, but I don't know how to do it or whether it's something we could do on the fly.)
> 
> Various sites are using 'let' a lot more.

That's unfortunate.  They claim some level of compliance with es6 and 7 but
I guess that they're not keeping up with the newer standards.  the latest I
could find was a standard from 2020 but the duktape feature comparison only
references up to about 2016 (and only with partial support apparently).
Given there're still active commits I wonder if there are plans to catch up
but I guess we may not have that time.
 
> We were working with dominos.com and they need postMessage.  We wrote a JS implementation of that one.

Interestingly, when I looked (briefly) at the latest es standard this was
only referenced as an example synchronisation mechanism, with the
implication that it'd be implemented as part of the browser rather than as
part of the js standard.  However, with some of this stuff that distinction
appears to be becoming rather... questionable.

> There's also the function bodies.  This one isn't ES6-related.  moz and v8 allow toString() of a function and return the source JS.  We think this is needed if we were to get ambitious and support paypal.com.  We spent a few weeks on it and solved several things but then ran into the buzzsaw of an obfuscated file they use which even has "traps".  And function bodies is part of what it's trying to detect.

That's too bad about the tostring.  It looks like there was a thought about
implementing this but I guess it never got fully implemented.  I remember
there was much discussion about what duktape does now in the case someone
runs this and I read some indications that they thought to implement a
function.source property to allow this.  However I've no idea if this was
ever done or if one needs custom config at build time (I could go further
into the code to find this but the yaml based config system which they use I
find... confusing).

> It's too bad.  duktape is terrific.  They don't not have the features,
> they're just someplace in the middle.  I think probably due to the massive amount of work it takes to incorporate them.

also because some of the features probably sit in that unfortunate place
between browsers and js engines.  When one develops both this is less
important because one can "make things work" but when one is using a js
engine which one doesn't develop this becomes somewhat more important.

I guess what I'm concerned about is the same concerns which motivated me to
suggest duktape all those years ago.  For example, Debian already packages
mozjs 52 and mosjs 78.  Apparently we (in our hello program), thanks to
Karl's dedication now have support for 52 and 60...  I'm hoping there's a
comparison between 60 and 78 in terms of API or how we keep up with this.

I'd rather we not be the reason that distros have to keep old versions of mozjs
around again.  However if that's what we have to do to keep a useful browser
then I'll do what I can to help.

Cheers,
Adam.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-16  7:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06  4:22 [edbrowse-dev] interwindow bleed Karl Dahlke
2020-11-09 23:11 ` Adam Thompson
2020-11-10  1:28   ` Karl Dahlke
2020-11-15 12:26     ` Adam Thompson
2020-11-15 12:45       ` Karl Dahlke
2020-11-16  3:27       ` Kevin Carhart
2020-11-16  7:54         ` Adam Thompson

edbrowse-dev - development list for edbrowse

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/edbrowse-dev

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 edbrowse-dev edbrowse-dev/ http://inbox.vuxu.org/edbrowse-dev \
		edbrowse-dev@edbrowse.org
	public-inbox-index edbrowse-dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.edbrowse.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git