Well even if I agree that we don't have to be bullet proof, I do think we
should keep such concerns in mind when designing our software, because a little
time spent here could prevent us having to invest much time in security fixes
in the future.
Chris
Von meinem Samsung Galaxy Smartphone gesendet.
-------- Ursprüngliche Nachricht --------
Von: Alex Harui <aha...@adobe.com>
Datum: 04.01.17 23:30 (GMT+01:00)
An: dev@flex.apache.org
Betreff: Re: [FlexJS] How to add html content?
There is a lot of incorrect assumptions in your email. I'm not going to
take the time to address all of them. It seems you are very passionate
about security, and it is great that you identified that directly setting
HTML into the DOM presents a security issue, so thank you for bringing
this up.
However, I disagree with the notion that everything we do must be
bullet-proof. Not everybody wants to or needs to ride around in a
military-grade vehicle. Go make the changes you want in the Express
component set, but please let others like myself create a low-level direct
access to the DOM for those who have appropriately sanitized their code
some other way. I will name it UnsecuredHTMLText.
My main priority is happy customers. Security, Functionality, and
Performance are trade-offs to reaching that goal. Realize that when you
build the team page, if we sanitize the team data some other way, all of
the security code you write will run and never catch anything. To me,
that's not a great example of PAYG.
I also don't want to put the burden on this group of volunteers right now
to try to meet an expectation that folks can write apps that are bullet
proof. If that's your itch, make a component set that tries to do that,
but please leave the rest of it alone.
Now if you or others are interested in leveraging our assets to try to
make a better API for catching/preventing more security issues, we can
have that discussion. I think there is potential to do more once we stop
thinking of HTML as a String.
My 2 cents. Other opinions welcome.
-Alex
On 1/4/17, 11:21 AM, "omup...@gmail.com on behalf of OmPrakash Muppirala"
<omup...@gmail.com on behalf of bigosma...@gmail.com> wrote:
>Before we lose context, I want add more of my thoughts here.
>
>I think the priority of things should be:
>
>1. Functionality
>2. Security
>3. Performance
>
>Alex probably thinks it should be:
>
>1. Functionality
>2. Performance
>3. Security
>
>As a compromise, in this context I am going to suggest that we go with:
>
>1. Security,
>2. Performance
>3. Functionality
>
>That is, security is still higher than functionality (which is what I
>want). And the priority of performance has not changed (I think what Alex
>wants)
>
>To explain it in a bit more detail: My goal is to not allow a developer
>to
>inadvertently add a bad piece of HTML to the DOM. That is how security
>exploits occur. We cannot simply say that it is upon the developer to
>ensure that a HTML snippet is sanitized before they use HTMLText to render
>it on the screen. We've seen this happen to Flash Player in the past
>where
>ease of use was prioritized over security. This behavior changed
>eventually, where security was considered when building each feature. I
>think we should learn from that to avoid being labeled as 'insecure
>software' .
>
>The options that Alex suggested (doing it in the HTTPService layer, etc.)
>does not cut it. Because:
>
>a. It is not appropriate for HTTPService to divine the nature of a piece
>of data. HTML can be used to display as DOM or shown to user to edit. We
>could have two UI widgets one, that shows the HTML in an editable text
>area, and one that previews the HTML. For example, editing an already
>comment in JIRA or a blog post and previewing it before saving. If we
>blindly let the HTTPService sanitize the data, we are losing data.
>b. A scary piece of HTML is not a security threat when it lives as a
>string. There is no need to do anything about it. It becomes a security
>threat only when it is added to the DOM. So, sanitizing it just before it
>is added to the DOM is the most appropriate thing to do. I guess this is
>perfectly adhering to the 'Pay as you go' philosophy. Don't sanitize the
>HTML just in case, sanitize it just in time.
>
>So, as a compromise, here is what I propose:
>
>By default, HTMLText component expects an IHTMLSanitizer bead. If such a
>bead is not available, the component will throw a runtime exception and it
>will not render the HTML.
>
>IHTMLSanitizer probably looks like this:
>
>function sanitizeHTML(v:String) :String;
>
>We will have two beads built in:
>1. HTMLSanitizer implements IHTMLSanitizer and uses a default algorithm
>to
>sanitize the given HTML.
>2. InsecureNoHTMLSanitizer implements IHTMLSanitizer which simply returns
>the same string back.
>
>We can build two 'express' components like this:
>
>express:HTMLText looks like this:
>
><basic:HTMLText>
> <basic:beads>
> <basic:HTMLSanitizer />
> </basic:beads>
><basic:HTMLText>
>
>express:InsecureHTMLText looks like this:
>
><basic:HTMLText>
> <basic:beads>
> <basic:InsecureNoHTMLSanitizer />
> </basic:beads>
><basic:HTMLText>
>
>This achieves the following goals:
>
>1. If someone wants to use basic:HTMLText and they don't provide a
>IHTMLSanitizer, they wont be able to display any HTML and will encounter
>runtime errors.
>2. They can use express:HTMLText which will run all HTML through a
>sanitizer. They will either be happy or will complain about performance.
>3. They can switch to express:InsecureHTMLText and get better
>performance. But since the name of the component clearly says that using
>it is insecure, we have at least given them an opportunity to understand
>what is going on.
>4. Some folks might want their own HTML sanitization logic. They can
>create their own SpecialHTMLSanitizer which implements IHTMLSanitize and
>add it to basic:HTMLText
>
>Thoughts?
>
>Thanks,
>Om
>
>P.S. Does the name 'TrustedHTMLText' work better than 'InsecureHTMLText'?
>
>On Tue, Jan 3, 2017 at 8:20 AM, flex capacitor <flexcapaci...@gmail.com>
>wrote:
>
>> I have a few thoughts on that. I need a component like this in the past
>>for
>> the times I had separate HTML content already created but needed to be
>> integrated into a Flex JS app. I would suggest a contentBefore,
>> contentAfter and contentOverride properties maybe as a bead or a full
>> separate component.
>>
>> Those would be child tag properties that would pass the raw content
>>through
>> enclosed in CDATA tags or not and the container or bead would have a
>> sanitize properties that could be set to true or false.
>>
>> The purpose would be to allow HTML embed content. Say you want to drop a
>> Google maps widget or rich text editor into your Flex JS app. You don't
>> want the compiler to parse that code. You would just want app to write
>>the
>> raw HTML to the document in the container it is declared in. You would
>>be
>> responsible for the security as the author and how to talk to it.
>>
>> The before and after properties would allow things like HTML comments or
>> other uses. In the past you had php code that wraps HTML markup but now
>>it
>> could be used for some of the other frameworks out there to use markup
>>to
>> perform other tasks.
>>
>> How that will work with the Flex JS model new App(); I'm not sure.
>>
>> On Jan 1, 2017 3:16 AM, "OmPrakash Muppirala" <bigosma...@gmail.com>
>> wrote:
>>
>> > On Sun, Jan 1, 2017 at 12:15 AM, Alex Harui <aha...@adobe.com> wrote:
>> >
>> > >
>> > >
>> > > On 12/31/16, 11:23 PM, "omup...@gmail.com on behalf of OmPrakash
>> > > Muppirala" <omup...@gmail.com on behalf of bigosma...@gmail.com>
>> wrote:
>> > > >
>> > > >Okay, I see your point now. To be clear, I did not set out to
>>write
>> > HTML
>> > > >code. The bio in team.json is a html snippet. So, I am just
>>trying
>> to
>> > > >display it as a piece of formatted data. Even in other projects,
>>I do
>> > > >need
>> > > >this kind of thing quite often.
>> > >
>> > > IMO, this is use case #1: non-interactive HTML content. We could
>>just
>> > > have an HTML component with an "html" property. But it would want
>>to
>> > wrap
>> > > the content in a <div> or <span>
>> > >
>> >
>> > This is all I wanted. I implemented this and it seems to work fine.
>> >
>> >
>> > >
>> > > Case #2: In MXML, someone wants to mix FlexJS elements with random
>> HTML.
>> > >
>> > > Case #3: In MXML, someone wants to put in random HTML where one
>>child
>> > HTML
>> > > tag has an id and wants to write code that addresses that id.
>> > >
>> > > The cheap option is to only support #1 by creating an HTML component
>> with
>> > > an "html" property. Everything else needs more thinking, IMO.
>> > >
>> >
>> > 2 and 3 sound like terrible ways to code :-)
>> >
>> >
>> > >
>> > > >
>> > > >That brings up a related concern. We need to sanitize such html
>> content
>> > > >during runtime [1]
>> > >
>> > > IMO, those are just utility functions. Not everybody needs
>> sanitization,
>> > > IMO, so PAYG.
>> > >
>> >
>> > Hmm, to play the devil's advocate, security should not be
>>pay-as-you-go.
>> > This should be opt-in by default. Someone will have to go the extra
>>mile
>> > to turn it off.
>> >
>> > This is the sort of thing that will go out in the wild and folks will
>>get
>> > affected by it soon enough. We will then need to push out an
>>emergency
>> > release to fix an XSS attack made possible by FlexJS.
>> >
>> > Either that or we call the default implementation 'InsecureHTMLText'
>>or
>> > something like that.
>> >
>> >
>> > >
>> > > >>
>> > > >>Not really sure why this is required. I am looking for usage
>> patterns
>> > > >>like
>> > > >>this:
>> > > >>
>> > > >><Panel>
>> > > >> <HTMLText>{myFancyHTMLText}</HTMLText>
>> > > >> <Image></Image>
>> > > >></Panel>
>> > > >>
>> > > >><TabNavigator>
>> > > >> <Tab>
>> > > >> <Button />
>> > > >> <HTMLText />
>> > > >> </Tab>
>> > > >> <Tab><HTMLText></Tab>
>> > > >></TabNavigator>
>> > > >>
>> > > >>I guess the HTMLText component has to be a UIBase for this to
>>work,
>> > > >>right?
>> > > >>
>> > > >>Maybe I am not understanding the point you are trying to make.
>> > > >
>> > > >The above would likely create an HTML DOM like:
>> > > >
>> > > ><Div className="Panel">
>> > > > <Span>the HTML from myFancyHTMLText</Span>
>> > > > <Img />
>> > > ></Div>
>> > > >
>> > > >
>> > > >Carlos asked earlier about getting rid of the Span since we all
>>know
>> it
>> > is
>> > > >legit to have the HTML DOM look like:
>> > > >
>> > > ><Div className="Panel">
>> > > > The HTML from MyFancyHTMLTExt
>> > > > <img />
>> > > ></div>
>> > > >
>> > > >
>> > > >I doubt if this HTML can be generated. Are you sure there will be
>>a
>> > > >linebreak after the text?
>> > >
>> > > No. I just added that to show the "children".
>> > >
>> > > >
>> > > >That looks a TextNode element. Which is lightweight than innerHTML
>> But
>> > > >are we sure we don't want formatting for such text? If we want to
>>be
>> > able
>> > > >to format, we need to use a p or a span element.
>> > >
>> > > AIUI, a TextNode is just plain text. If the above was:
>> > >
>> > > <div className="Panel">
>> > > The <b>HTML</b> from <strong>MyFancyHTMLTExt</strong>.
>> > > <img />
>> > > </div>
>> > >
>> > > There would be a TextNode for "The", a Bold node for "HTML", another
>> > > TextNode for "from" etc.
>> > >
>> >
>> > What I meant was that HTML could be achieved by using the
>> > document.createTextNode() function which creates a TextNode element.
>> i.e.
>> > no need for a wrapping element like P or Span. Sorry for the
>>confusion.
>> >
>> >
>> > >
>> > >
>> > > This doesn't feel right to me. All of us seem tempted to want to
>> define
>> > a
>> > > scrap of complex HTML separate from other child controls in a
>> container.
>> > > If there was an HTML control, it would still add a wrapping element.
>> If
>> > > you pull of a trick where the HTML control just injects the content,
>> you
>> > > run into more complexity dealing with numElements and childIndex.
>>It
>> > > would be what the browser says it would be, but not quite as obvious
>> from
>> > > the MXML.
>> > >
>> > > On the other hand we have those thin-wrapping components like Div,
>>and
>> A
>> > > and you ought to be a able to put in HTML content without it being
>> > wrapped
>> > > by another layer.
>> > >
>> >
>> > Yes, I am in agreement with you here. A div or span with innerHTML is
>> more
>> > than adequate for this usecase.
>> >
>> >
>> > >
>> > > >There is a cost in both development time and runtime performance to
>> > > >generalizing this. Should we do it?
>> > > >
>> > > >
>> > > >Hmm, I don't think this requires a change to MXML parsing.
>> > >
>> > > I think it does. The default property is expected to be an array of
>> > > Ichild elements. When you have random HTML in the default property
>>the
>> > > compiler won't accept XML TextNodes in many places and where it
>>does,
>> it
>> > > tries to concatenate TextNodes which is what we don't want.
>> > >
>> >
>> > I see. I did not realize this point. In any case, I would advice not
>> > adding a random TextNode element like this. This also brings more
>> security
>> > issues [1]
>> >
>> > Thanks,
>> > Om
>> >
>> > [1] http://benv.ca/2012/10/02/you-are-probably-misusing-DOM-
>> text-methods/
>> >
>> >
>> >
>> > >
>> > > Other opinions welcome...
>> > > -Alex
>> > >
>> > >
>> >
>>
