1 Pages  1 2 3  
Reply to this topic  Start new topic
> ** Security Notice **, Important security news for developers
Rob Marquardt
post Aug 3 2007, 01:35 AM
Post #1


I have a spoon.



Posts: 1,816
Joined: 12-February 03
From: California - Bay Area
Member No.: 289



Security is very important to us, and it should be for you too. Making sure that Yahoo! Widgets users can trust what they get from the Yahoo! Widget Gallery is an ongoing priority here in Widgetland. Recently, it came to our attention that a legacy Widget-building practice made the Widgets built in that manner vulnerable to a potentially severe security exploit. Since then, we've been taking a number of steps to secure the affected Widgets built by both Yahoo! and other authors. As of this post, all of the Widgets in which we detected this vulnerability have been removed from the public-facing section of the Yahoo! Widget Gallery and are no longer available for download. And if your Widget has been removed, you should be receiving an email with instructions on how to have your Widget reinstated.

Potential vulnerability in Widget event handlers and how to make them more secure

Historically (that is, before Konfabulator 3) there was only one way to assign an event handler to an object, and that was by assigning a string.

CODE
image.onMouseUp = "alert( 'You clicked me!' )";

text.onMouseEnter = "myTextFunction( )";

This was always a bit of a wonky way to do things. A common task was to construct that string at runtime, often with data from an outside source -- say, to open a browser window from an address contained in an RSS feed.

CODE
text.onMouseUp = "openURL( '" + urlFromFeed + "' )";

You can probably see where this is heading. It's possible for a malformed address to get passed in and added to the string. This would then execute in response to the event handler and potentially cause the Widget to act in unexpected ways.

Fortunately, there's an easy way around this. As of Konfabulator 3, functions can be used as event handlers instead of strings. Here's an example of how you could assign that address at runtime in an object-oriented way.

First, turn on custom object attributes in your Widget's <settings> block:

CODE
<settings>
    <setting name="allowCustomObjectAttributes" value="true"/>
</settings>

Now, instead of building the event handler to open the URL as a string, we'll assign the address to a new attribute of the object and then point the event handler to a function:

CODE
text.href = urlFromFeed;
text.onMouseUp = openMyUrl;

Finally, we'll make the function that does the work. First the function checks to see if the object that called it contains our custom "href" attribute, and if so, passes it to openURL( ).

CODE
function openMyUrl( )
{
    if( this.href )
        openURL( this.href );
}

This function could be used as the event handler for any object -- Text, Image, Frame, etc -- that you added the custom "href" attribute to. Note that there's nothing special about using "href" as the name of the custom attribute. It just has to be different from the object's existing attribute names.

If you only needed to do this task once, you could also do it as an anonymous function:

CODE
text.onMouseUp = function ( )
{
    openURL( urlFromFeed );
}

Using functions over strings as event handlers is not only good coding practice to do today, it will be the required way with Konfabulator versions going forward. In addition to the "this" object, the next major Konfabulator version will also give your function direct access to its calling event, without having to use system.event.

Another, albeit poorer, solution

If your Widget absolutely needs to run on Konfabulator versions prior to 3.0 (really? really?), you can also filter strings coming in from outside sources to prevent potentially malicious code from executing.

First, if it's not already, be sure your Widget's onLoad handler is wrapped in a CDATA block:

CODE
<action trigger="onLoad"><![CDATA[

    // Your code here

]]></action>

Then, add this function:

CODE
function removeInjectionsForWeb( str )
{
    if( !str )
        return;

    str = String( str );

    str = str.replace( /[<>\"\'\\\n\r]/g,
        function( c )
        {
            c = escape( c );
            return c;
        }
    );

    return str;
}

You can then use this function to validate text coming from outside sources, like so:

CODE
text.onMouseUp = "openURL( '" + removeInjectionsForWeb( urlFromFeed ) + "' )";

This should be treated as more of a stop-gap measure until you can rewrite your Widget to use functions to handle events.

Widgets that use text from outside sources in an event handler as a string without validation will be rejected from the Gallery outright. Widgets that use validated text in an event handler as a string will take longer to process and approve than those that use functions. And as mentioned before, future versions of Konfabulator will require functions to be used as event handlers.

An event handler summation:

Strings: Potential to execute arbitrary code, inflexible, going away.

Functions: Does the filtering for you, flexible, the one true way.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Indiana
post Aug 3 2007, 07:56 AM
Post #2






Posts: 679
Joined: 16-November 05
From: Munich, Germany
Member No.: 11,213



Your filter for removing widgets is to strange.
I do the following in my removed widgets:
CODE
var tmp = "...a url ...";
url.open(tmp);

The reason why I do that? I have more the one url for testing so it was easier just to comment out the unused versions.
But ok .. i will overwork it and direct copy the url as string into the open function.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Herbert
post Aug 3 2007, 09:05 AM
Post #3






Posts: 8
Joined: 19-June 07
Member No.: 22,766



As far as i can see this is only for 'OpenUrl', right?

Or is the fetch() a subject to change too ?

Herbert




***
Author of AC-Search
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Indiana
post Aug 3 2007, 09:47 AM
Post #4






Posts: 679
Joined: 16-November 05
From: Munich, Germany
Member No.: 11,213



So, now I updated my first one ... hope they will now be accepted.
I'm anxious when it will be on the gallary.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Drevor
post Aug 3 2007, 03:01 PM
Post #5






Posts: 2,234
Joined: 28-May 05
From: 51.0°N 13.7°E
Member No.: 6,247



This is not about openURL but about any sort of function or eval construction that uses remote content.

I guess a example is on order. The following piece of code is supposed to call the function fubar() with the content of foo as a parameter. The old and dangerous code would look like this
CODE
myButton.onMouseUp = "fubar( '" + foo + "' )";


What would happen if foo would not just contain a simple string but this:
CODE
hello there' , runCommand("format C:") , '
The constructed function would not just get one parameter but three. Two strings and the result of runCommand("format C:"), which as I understand, would be a rather disgruntled user.

Thus, always check the content of anything you receive. Url.fetch, onWidgetTell or just a 3rd party function library -be skeptic. Its better to check once to often than once to few.

And as always, the best method its to write proper code not quirks wink.gif




. . .
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Rob Marquardt
post Aug 3 2007, 04:59 PM
Post #6


I have a spoon.



Posts: 1,816
Joined: 12-February 03
From: California - Bay Area
Member No.: 289



... what Drevor said.

openURL is a common culprit for this, but it's any event handler built as a string containing data from outside sources.

The minimumVersion flag on the next major Konfabulator release won't allow strings to be used as event handlers at all.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Indiana
post Aug 3 2007, 07:46 PM
Post #7






Posts: 679
Joined: 16-November 05
From: Munich, Germany
Member No.: 11,213



I think you don't check the code manully.
Why doesn't you add the result to the email?
Ther must be a result like ...
CODE
xyz.kon (line 123) used string as event handler.

That whould help very much!




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
ghostwalker
post Aug 3 2007, 08:47 PM
Post #8






Posts: 1,967
Joined: 12-November 04
From: Seneca, Missouri
Member No.: 3,762



Or a widget that we can drop our widget on to find all instances would also be nice.




Widgets : IPB Image Have a nice day | IPB Image Today Is | IPB Image FileInfo | IPB Image Smooth Clock
In Development : IPB Image My Gmail | IPB Image WildBlue Usage
Co-Development : IPB Image KonAvio Media |IPB Image KonText-Plain Text Editor
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Drevor
post Aug 3 2007, 09:06 PM
Post #9






Posts: 2,234
Joined: 28-May 05
From: 51.0°N 13.7°E
Member No.: 6,247



QUOTE(ghostwalker @ Aug 3 2007, 10:47 PM) *
Or a widget that we can drop our widget on to find all instances would also be nice.
not really possible .onSomething=fubar is ok it fubar is a function but not if it is a string. You cant test that with text pattern. Just go through your code and find all .on* and see what you are handing them over. Or wait for the next release of K and see where it complains.

The only *automated* thing you could do is write a function that walks up and down the DOM tree and all global js variables and checks all event handlers. But this will not catch objects that are beein created at a different point in time and things like the url object.


Do it yourself, its your code, you should know where it does what.

//edit
Any code is vulnerable this way. Do not just look a the event handlers. Check where you fetch data from external sources. Test the variables for type and content. Utilize closure.
CODE
var myUrl=theUrl;
myImage.onClick= function() { openURL(myUrl) };
works just as fine but without allowCustomObjectAttributes
(allowCustomObjectAttributes stops the engine from complaining whenever you made a typo - myImage.heigth anyone wink.gif ).




. . .
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Rob Marquardt
post Aug 3 2007, 09:50 PM
Post #10


I have a spoon.



Posts: 1,816
Joined: 12-February 03
From: California - Bay Area
Member No.: 289



QUOTE(Drevor @ Aug 3 2007, 02:06 PM) *
CODE
var myUrl=theUrl;
myImage.onClick= function() { openURL(myUrl) };

works just as fine but without allowCustomObjectAttributes

As mentioned above in the anonymous function example.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Brendan Bellina
post Aug 3 2007, 11:56 PM
Post #11






Posts: 45
Joined: 28-June 06
From: Los Angeles, CA
Member No.: 15,845



"If we do not receive a response from you by August 16, the current version of your
Widget will be disabled from running on other peoples' computers and
you will need to update your Widget in order to get it back in the
Gallery and running on peoples' desktops again."

I think the heavy-handed handling of this is particularly concerning. To make announcements that widget authors should avoid insecure coding practices is fine, as is releasing a new version of the Engine that disallows those practices, or rejecting new gallery submissions that have them. Removing widgets from the gallery is too much. And disabling widgets so that they will no longer operate whether a user wants you to or not is going way way too far.

You guys need to drop the gestapo tactics or developers are going to drop you.

I, for one, am rapidly losing interest because of these tactics.
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
BOINCer
post Aug 4 2007, 12:05 AM
Post #12






Posts: 1,288
Joined: 28-September 05
From: Buenos Aires, Argentina
Member No.: 10,111



In my opinion, removing widgets from the gallery is fine, but disabling a widget on users' computers should only be done after asking the user.

And on widgets using strings in a way that does NOT compromise the security, like Indiana's earlier on this thread, it shouldn't bother the user at all. No disabling, no warning, nothing.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
ghostwalker
post Aug 4 2007, 12:20 AM
Post #13






Posts: 1,967
Joined: 12-November 04
From: Seneca, Missouri
Member No.: 3,762



I have to agree disabling widgets after you already inform them on first run that this widget is not made by Yahoo and "You should only use this Widget if you know and trust the source." This should be enough, removing them from the gallery I think is fine. But now days the Widget could be downloaded from other places besides the gallery and disabling it would be a bad idea.




Widgets : IPB Image Have a nice day | IPB Image Today Is | IPB Image FileInfo | IPB Image Smooth Clock
In Development : IPB Image My Gmail | IPB Image WildBlue Usage
Co-Development : IPB Image KonAvio Media |IPB Image KonText-Plain Text Editor
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Jonathan S.
post Aug 4 2007, 01:02 AM
Post #14






Posts: 85
Joined: 15-October 06
From: Sunnyvale, CA
Member No.: 17,171



QUOTE(ghostwalker @ Aug 3 2007, 05:20 PM) *

I have to agree disabling widgets after you already inform them on first run that this widget is not made by Yahoo and "You should only use this Widget if you know and trust the source." This should be enough, removing them from the gallery I think is fine. But now days the Widget could be downloaded from other places besides the gallery and disabling it would be a bad idea.


The issue is that a vulnerable Widget could potentially be hijacked to make our platform do really bad stuff to a user's machine. That is unacceptable to us, regardless of where the Widget came from or to what the user agreed. The source of the Widget might not be malicious, but if the Widget author didn't know any better, he/she may have left the door open to someone not so well-intentioned and not at all trusted by the user.

QUOTE(BOINCer @ Aug 3 2007, 05:05 PM) *

In my opinion, removing widgets from the gallery is fine, but disabling a widget on users' computers should only be done after asking the user.


Again, the user may have no idea what could potentially happen if they don't disable the Widget. We would love to have more granular options than this, and we're already planning to add them in the future. But right now, we have no means to notify users of the security issue with their Widgets other than to disable them. And, for the reasons stated above, we must err on the side of caution.

QUOTE(BOINCer @ Aug 3 2007, 05:05 PM) *

And on widgets using strings in a way that does NOT compromise the security, like Indiana's earlier on this thread, it shouldn't bother the user at all. No disabling, no warning, nothing.


As Ricky said in his email, if you feel your Widget was flagged incorrectly for removal, please let us know via support and we will double-check it.

We know this sucks for a lot of you. And we're truly sorry for the inconvenience. We spent a lot of time discussing this, and feel that this is the best possible solution to a highly shitty situation (though we haven't been sleeping too much since this started, so who knows if we're in our right minds). And fwiw, we're committed to making sure that things are less painful if something like this does happen again in the future.

Thanks for your support.
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Rob Marquardt
post Aug 4 2007, 01:06 AM
Post #15


I have a spoon.



Posts: 1,816
Joined: 12-February 03
From: California - Bay Area
Member No.: 289



QUOTE(Brendan Bellina @ Aug 3 2007, 04:56 PM) *
I think the heavy-handed handling of this is particularly concerning. To make announcements that widget authors should avoid insecure coding practices is fine, as is releasing a new version of the Engine that disallows those practices, or rejecting new gallery submissions that have them. Removing widgets from the gallery is too much. And disabling widgets so that they will no longer operate whether a user wants you to or not is going way way too far.

Yes, it's heavy-handed. It's a serious issue. Yahoo takes security issues seriously.

Which would you rather have? "My users had to download a new version of my Widget" or "As a result of running my Widget, a user had his hard drive wiped"?

QUOTE(BOINCer @ Aug 3 2007, 05:05 PM) *
In my opinion, removing widgets from the gallery is fine, but disabling a widget on users' computers should only be done after asking the user.

"The Widget you're about to run has a known security vulnerability that could be exploited by a remote attacker to destroy all data on your computer."
[Allow]   [Cancel]

QUOTE(BOINCer @ Aug 3 2007, 05:05 PM) *
And on widgets using strings in a way that does NOT compromise the security, like Indiana's earlier on this thread, it shouldn't bother the user at all. No disabling, no warning, nothing.

The Widgets that were removed from the public-facing portion of the Gallery were done so by an automated process. They're "on hold." They haven't been deleted. I do not know the algorithm that was used, but those Widgets that got a false-postive will be reinstated.

Again, this is a serious issue. Every attempt is being made to make things as painless as possible, but no question that this is a major inconvenience to the Widgets team, third-party developers and Widget users. But at the end of the day, the only thing that matters is the security and safety of Widget user's computers.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Jonathan S.
post Aug 4 2007, 01:23 AM
Post #16






Posts: 85
Joined: 15-October 06
From: Sunnyvale, CA
Member No.: 17,171



Hehe, Rob and I share a brain.

Wait, that's not funny!
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
ghostwalker
post Aug 4 2007, 01:31 AM
Post #17






Posts: 1,967
Joined: 12-November 04
From: Seneca, Missouri
Member No.: 3,762



QUOTE
The issue is that a vulnerable Widget could potentially be hijacked to make our platform do really bad stuff to a user's machine. That is unacceptable to us, regardless of where the Widget came from or to what the user agreed. The source of the Widget might not be malicious, but if the Widget author didn't know any better, he/she may have left the door open to someone not so well-intentioned and not at all trusted by the user.


In that case it is totally understandable


QUOTE
"The Widget you're about to run has a known security vulnerability that could be exploited by a remote attacker to destroy all data on your computer."
[Allow] [Cancel]


The obvious choice

After careful thought I believe you are doing the correct thing here even though it may an inconvenience to the widget author, sorry I questioned your choices.




Widgets : IPB Image Have a nice day | IPB Image Today Is | IPB Image FileInfo | IPB Image Smooth Clock
In Development : IPB Image My Gmail | IPB Image WildBlue Usage
Co-Development : IPB Image KonAvio Media |IPB Image KonText-Plain Text Editor
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
Brendan Bellina
post Aug 4 2007, 01:42 AM
Post #18






Posts: 45
Joined: 28-June 06
From: Los Angeles, CA
Member No.: 15,845



QUOTE(Rob Marquardt @ Aug 3 2007, 06:06 PM) *

Yes, it's heavy-handed. It's a serious issue. Yahoo takes security issues seriously.

Which would you rather have? "My users had to download a new version of my Widget" or "As a result of running my Widget, a user had his hard drive wiped"?

Many Widget developers develop in their spare time. Your algorithm has flagged - I believe inappropriately - 28 of my widgets. If modifications are required I will not have enough spare time in 13 days to examine, test, and update all of them by August 16. As a result potentially thousands of users of my widgets will be emailing me wondering why as far as they know they changed nothing on their systems and yet my widgets are suddenly not working. I'll be sure to forward their emails to the Yahoo support team so you can explain why you changed the behavior of programs on their systems in their "best interest" without their knowledge and agreement (some call that a virus I believe). I'm sorry the Windows operating system is inherently insecure, but this doesn't really fix that and handling it the way Yahoo has chosen to do is going to result in ticking off a lot of users and developers.

This is only the latest in a series of heavy-handed changes that have undermined my trust in the judgment of your team. At this point I don't know what absurdity to expect next.

No problem, I can rewrite my widgets in Dashboard. Thanks fror your time.
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
BOINCer
post Aug 4 2007, 01:47 AM
Post #19






Posts: 1,288
Joined: 28-September 05
From: Buenos Aires, Argentina
Member No.: 10,111



This has *nothing* to do with Windows security; it's just Javascript security when widgets are coded that way. Macs are just as vulnerable.

And you don't need to update anything if you check the widgets and you find they aren't really insecure (like Indiana's), just mail back saying the algorithm got a false positive and they will review it manually.




User is offlineProfile CardPM
Go to the top of the page
+Quote Post
bjbk
post Aug 4 2007, 02:29 AM
Post #20






Posts: 510
Joined: 6-August 05
Member No.: 8,181



For widgets not uploaded to the gallery, will they also stop working on August 16th?
User is offlineProfile CardPM
Go to the top of the page
+Quote Post
1 Pages  1 2 3 
Fast Reply  Reply to this topic    Start new topic  
1 User(s) are reading this topic (1 Guests and 0 Anonymous Users)
0 Members: