Bug 238860 - [Commands] Expressions should be protect-able against 3rd party modifications of their variables
Summary: [Commands] Expressions should be protect-able against 3rd party modifications...
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows Vista
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-06-27 18:34 EDT by Jan-Hendrik Diederich CLA
Modified: 2019-09-06 15:36 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan-Hendrik Diederich CLA 2008-06-27 18:34:23 EDT
Build ID: I20080617-2000

Steps To Reproduce:
1. Create, for example, an expression-based activity. An activity which enablement is controlled by an expression. An expression which depends, e.g., on a variable (e.g. "rightsVariable", which must have the value "allowAll").
A variable which is controlled by a SourceProvider.

2. Now create a "compromising plugin" which registers also a SourceProvider, which provides the same variable with the same value.

Register this source provider and fire a "source changed", so the compromisers variable value gets into the system, even if the variable, corresponding to the state of the original plugin, shouldn't be set.

More information:
Expression-based activities are now also used to control UI elements, to remove/add them depending on authorization states. So there should be a possibility to make it sure that at least the variables of expressions can only be set by the author (if he wishes(!)), and not by arbitrary 3rd party plugins.

I suggest you add the option to set a password attribute to the expression variable value test (the "with" node), that every SourceProvider must provide.  A password that is stored in the attribute in its encrypted state (like the Linux password file). But I admit I'm no security expert and have only vague ideas how to store the the un-encrypted password in the system, that the SourceProvider must transmit.

Remarks:
- For the beginning it doesn't have to be super-secure, but at least there should be some _major_ obstacle, and not like now, where everybody can shortly drop by and circumvent it just on the fly.

- I chose [expression] variables because the programmers seem to favoring these for controlling expressions, although there are other methods (property testers for example - but they have the problem that extension point entries can be overwritten by other entries with the same id).
Comment 1 Paul Webster CLA 2008-07-02 08:36:37 EDT
The way we normally provide some control over extensions (IExtensionRegistry will return extensions in no particular order) is we sort them by contributing bundle ID before processing.  Bundles that are lexigraphically later that provide extensions (or extension elements) with IDs that have already been read are ignored.

This isn't security, by any stretch of the imagination, simply predictability.

The best option to do something like this in 3.4 would be for an RCP app to use the Product Customization and insure that only the approved source providers are read (remove any source providers that aren't known).


PW
Comment 2 Jan-Hendrik Diederich CLA 2008-07-02 09:56:58 EDT
(In reply to comment #1)
> The best option to do something like this in 3.4 would be for an RCP app to use
> the Product Customization and insure that only the approved source providers
> are read (remove any source providers that aren't known).

You mean it should get an attribute "allowed="com.company.bla.x/whateverId;com.company.bla.x/somethingId""? That would be a good way to declare a restriction directly into the xml code, so it's a reliable and transparent solution. What do you think?
Comment 3 Paul Webster CLA 2008-07-02 10:17:32 EDT
Not exactly ... I mean something like an XSLT transform that the RCP app could use.

for example, with the current org.eclipse.equinox.transforms.xslt you could use:
#Bundle ID regex, Resource Path regex, resource path
org\.eclipse\.ui,plugin\.xml,/identityTransform.xslt
.*,plugin\.xml,/filterSource.xslt

Where filterSource.xslt would have code like:

 <xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
     <xsl:template match="sourceProvider[variable/@name='selection']">
     </xsl:template>
     <xsl:template match="node()|@*">
         <xsl:copy>
             <xsl:apply-templates select="node()|@*"/>
         </xsl:copy>
     </xsl:template>
 </xsl:stylesheet>

(or at least something close to that, my XSL/XPath is rusty).  This would remove any sourceProvider that "contributed" selection that was not in the org.eclipse.ui/plugin.xml file.

PW
Comment 4 Jan-Hendrik Diederich CLA 2008-07-02 10:41:11 EDT
(In reply to comment #3)
> Not exactly ... I mean something like an XSLT transform that the RCP app could
> use.
That's also a solution. But this XSLT transformation is again somehow a hack. If If I would provide a sensitive patch, would you also accept that?
(A patch that goes the way I suggested in Comment #2.)
And if you would accept it, would it be integrated in Eclipse 3.4.1 or 3.5?
Comment 5 Jan-Hendrik Diederich CLA 2008-07-02 10:42:38 EDT
(In reply to comment #4)
> ... If I would provide a ""_sensitive_"" patch ...
*sensible*   of course, sorry
Comment 6 Paul Webster CLA 2008-07-02 13:24:55 EDT
(In reply to comment #4)
> (In reply to comment #3)
> > Not exactly ... I mean something like an XSLT transform that the RCP app could
> > use.
> That's also a solution. But this XSLT transformation is again somehow a hack.
> If If I would provide a sensitive patch, would you also accept that?


No.  Controlling extensions is a general problem.  It should probably be solved at the product level (as with the transforms already provided) or at the IExtensionRegistry level (which would probably require significant equinox feature work).

PW
Comment 7 Jan-Hendrik Diederich CLA 2008-07-02 13:48:28 EDT
(In reply to comment #6)
> No.  Controlling extensions is a general problem.  It should probably be solved
> at the product level (as with the transforms already provided) or at the
> IExtensionRegistry level (which would probably require significant equinox
> feature work).

I must make sure we're not cross-talking, but I think we're talking about two different problems that result in the same thing: hijacking through double declarations/implementations.

1. You can re-declare extension point declarations in exsd files. So you can overwrite the original "org.eclipse.ui.whatever.exsd" files.
That must be addressed in Equinox itself. Full ACK.

2. I can define another SourceProvider which provides variables for Expression tests, which aren't intended to be modified by another programmer than the first one. So Mr. Millers expressions test for "xy", which isn't set, and I come along and set it - which is _extreme_ easy. So the whole state of Mr. Millers plugin gets screwed up.

I know that (1) is a major problem, but after thinking about it, and how much code-change, and how deep this must be done, I decided to set my focus on the easier and quicker to solve problem. A problem which won't be solved by solving (1) anyway.

Isn't Comment #2 an acceptable solution for (2)?
Comment 8 Jan-Hendrik Diederich CLA 2008-07-02 14:12:32 EDT
(In reply to comment #7)

> 1. You can re-declare extension point declarations in exsd files. So you can
> overwrite the original "org.eclipse.ui.whatever.exsd" files.
> That must be addressed in Equinox itself. Full ACK.

Ehmmm... sorry. You can do that, that's a problem. But you also can re-declare the usage of the exsd files in your plugin.xml files with using the same extension id. That would also overwrite the declaration. This is the major problem, I really ment.
Comment 9 Paul Webster CLA 2008-07-02 14:18:51 EDT
(In reply to comment #7)
> Isn't Comment #2 an acceptable solution for (2)?

Not really.  I'm not interested in the swap-out (1) case ... that's an equinox/signed bundles/product problem.

I'm specifically talking about (2).  This is not a source provider/expression problem.  This is fundamental to our extension registry.  When elements have the same ID, which wins?  First in?  Last in?  Random extension registry order or extension point specific order?

Given how different Platform UI extension points have failed to provide even predictability using some of these policies, I think that security+extension needs to be solved properly, at a much lower level.

However, as a product/RCP app can control what comes in and using XSLT transforms prevent certain items from showing up, the transform approach is good enough to prevent other plugins from trying to override extensions important to that product.  It's more secure than anything we could hack in at the extension level, and can be applied to as many extension points as that product deems necessary.

PW
Comment 10 Jan-Hendrik Diederich CLA 2008-07-02 15:41:46 EDT
(In reply to comment #9)
I would like to quote from the IRC discussion that you can also add a source provider via a plugin.xml declaration. While to my knowledge the _standard_ solution to add a SourceProvider (from to the tutorials I read about this issue), is to do it programmatically, this isn't a problem. Even if it's added programmatically you can remove any instances of ISourceProvider via Equinox transformations, although they have no entries in any plugin.xml files.
Comment 11 Eclipse Webmaster CLA 2019-09-06 15:36:30 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.