Hello Ido,

  The project has had a lot of contributors over the years, it is
probably not safe to assume they have all been "experts" and
frequently "experts" know of little tricks or shortcuts that can
result in a lot of trouble (this case in point) - a less sophisticated
developer probably would not have even known about the Python
functionality that resulted in this trouble.

  I suspect the mere fact that you are concerned about writing secure
tools means you would do a better job than many professional software
developers whom you may term as "experts". Furthermore, you are
absolutely right there should be some documentation or something
somewhere to aid in writing secure tools - the rest of this e-mail
contains a couple quick notes hopefully it can be translated to the
wiki at some point and grown over time.

  If your tool is not taking in text inputs - its all numbers and
select parameters, etc.... it is very likely secure. These sorts of
vulnerabilities would usually come into play when users are allowed to
pass free text and the tool or wrapper use this text in such a way
that it can be broken out of the intended context (these are broadly
characterized as code injection attacks). 95% of how these text
parameters are going to be used is likely passing them as a
command-line argument to another program. For this reason Galaxy
preprocesses the text and sanitizes it so it cannot contain characters
that would result in the text easily result in code injections.

  So for this reason - you are still probably fine unless you are
"circumventing" this text preprocessing. For instance, Galaxy will
translate quotations marks to '__dq__', this tool explicitly
retranslates those back to quotations marks
(https://bitbucket.org/galaxy/galaxy-central/src/f2f1cce4678cf1eb188d9611b05f00706afc8897/tools/stats/filtering.py?at=default#cl-176).
There is a reason to do this in this case but you will not need for
most bioinformatics applications. If your tools are doing this it is
time to start getting extra careful.

  If you are really interested in this topic or when it is time to get
extra careful, I would recommend picking up the book "The Web
Application Hacker's Handbook" - it is pretty good. Most of it would
not be relevant for tool developers, but chapter 1, chapter 2, and all
of chapter 9 could be very relevant and would probably leave one with
a solid grasp of what to look for in many different contexts - not
just the ones the book discusses explicitly.

  Hopefully over time the IUC will provide guidance about this sort of
thing (informing you if there are potential security vulnerabilities
in your tools). Also feel free to post example tool configurations to
this list you might be concerned about and I am sure someone would be
happy to look it over and tell you if there are any red flags.

-John

On Tue, Nov 5, 2013 at 12:30 PM, Ido Tamir <tamir@imp.ac.at> wrote:
>
> On Nov 5, 2013, at 6:28 PM, Nate Coraor <nate@bx.psu.edu> wrote:
>
>> Hi Ido,
>>
>> Thanks for the feedback.  Replies below.
>>
>> On Nov 5, 2013, at 9:54 AM, Ido Tamir wrote:
>>
>>> This seems to happen often e.g. http://wiki.galaxyproject.org/DevNewsBriefs/2012_10_23#Compute_Tool_Security_Fix
>>
>> I'm not sure I'd agree that it's often - we've had 4 or 5 vulnerabilities over the life of the project.  2 allowed arbitrary code execution, the others were less severe.
>>
> But these were written by experts, not by people like me, that don't know what the galaxy framework really does/does not do with the input, so I guess I make many more mistakes.
>
>>> a) are there general guidelines in the wiki on how to avoid these problems when creating tools?
>>
>> The guidelines for writing a Galaxy tool are no different from best practices for writing secure code.  In specific for this vulnerability, execution of user input should be handled with extreme care, and this tool had some gaps in its input validation and sanitization.  For what it's worth, the filter tool (on which the other vulnerable tools were based) is one of the few tools surviving from the very early days of Galaxy, and would not be implemented the same way if written today.
>
> I think it would be nice to have a small outline on the wiki of what galaxy does with the input data and how it could affect a tool.
> What sanitisation is there by default so I don't have to worry about it, but what could happen I if I don't care to check/remove sanitise ' | or " ..., maybe with examples.
>
>>> b) is there a way to check automatically if all input fields are correctly escaped in a tool?
>>
>> I am not sure how Galaxy could do this.  Galaxy sanitizes the command line so that input fields passed to a tool as command line arguments cannot be crafted to exploit the shell's parsing rules.
> Thats good
>
> best,
> ido
>
>
>> What the tool itself does with its inputs are out of Galaxy's control.
>>
>> --nate
>>
>>>
>>> A search for security in the wiki brings up:
>>>      • Admin/Data Libraries/Library Security
>>> 0.0k - rev: 1 (current) last modified: 2013-01-02 23:54:33
>>>      • Admin/DataLibraries/LibrarySecurity
>>> 19.2k - rev: 4 (current) last modified: 2013-01-03 00:12:36
>>>      • HelpOnConfiguration/SecurityPolicy
>>> 1.9k - rev: 1 (current) last modified: 0
>>>      • Learn/Security Features
>>> 7.0k - rev: 3 (current) last modified: 2011-09-13 16:52:08
>>>      • News/2013_04_08_Galaxy_Security_Release
>>> 1.3k - rev: 3 (current) last modified: 2013-04-08 16:56:41
>>>
>>> "escape" does not bring up anything.
>>>
>>> thank you very much,
>>> ido
>>>
>>>
>>> On Nov 5, 2013, at 12:45 AM, Nate Coraor <nate@bx.psu.edu> wrote:
>>>
>>>> A security vulnerability was recently discovered by John Chilton with Galaxy's "Filter data on any column using simple expressions" and "Filter on ambiguities in polymorphism datasets" tools that can allow for arbitrary execution of code on the command line.
>>>>
>>>> The fix for these tools has been committed to the Galaxy source.  The timing of this commit coincides with the next Galaxy stable release (which has also been pushed out today).
>>>>
>>>> To apply the fix and simultaneously update to the new Galaxy stable release, ensure you are on the stable branch and upgrade to the latest changeset:
>>>>
>>>> % hg branch
>>>> stable
>>>>
>>>> % hg pull -u
>>>>
>>>> For Galaxy installations that administrators are not yet ready to upgrade to the latest release, there are three workarounds.
>>>>
>>>> First, for Galaxy installations running on a relatively new version of the stable release (e.g. release_2013.08.12), Galaxy can be updated to the specific changeset that that contains the fix.  This will include all of the stable (non-feature) commits that have been accumulated since the 8/12 release plus any new features included with (and prior to) the 8/12 release, but without all of the new features included in the 11/4 release.  Ensure you are on the stable branch and then upgrade to the specific changeset:
>>>>
>>>> % hg pull -u -r e094c73fed4d
>>>>
>>>> Second, the patch can be downloaded and applied manually:
>>>>
>>>> % wget -o security.patch https://bitbucket.org/galaxy/galaxy-central/commits/e094c73fed4dc66b589932edb83412cb8b827cd3raw/
>>>>
>>>> and then:
>>>>
>>>> % hg patch security.patch
>>>>
>>>> or:
>>>>
>>>> % patch -p1 < security.patch
>>>>
>>>> Third, the tools can be completely disabled by removing them from the tool configuration file (by default, tool_conf.xml) and restarting all Galaxy server processes.  The relevant lines in tool_conf.xml are:
>>>>
>>>> <tool file="stats/dna_filtering.xml" />
>>>> <tool file="stats/filtering.xml" />
>>>>
>>>> The full 11/4 Galaxy Distribution News Brief will be available later today and will contain details of changes since the last release.
>>>>
>>>> --nate
>>>> Galaxy Team
>>>> ___________________________________________________________
>>>> Please keep all replies on the list by using "reply all"
>>>> in your mail client.  To manage your subscriptions to this
>>>> and other Galaxy lists, please use the interface at:
>>>> http://lists.bx.psu.edu/
>>>>
>>>> To search Galaxy mailing lists use the unified search at:
>>>> http://galaxyproject.org/search/mailinglists/
>>>
>>>
>>
>
>
> ___________________________________________________________
> Please keep all replies on the list by using "reply all"
> in your mail client.  To manage your subscriptions to this
> and other Galaxy lists, please use the interface at:
>   http://lists.bx.psu.edu/
>
> To search Galaxy mailing lists use the unified search at:
>   http://galaxyproject.org/search/mailinglists/

___________________________________________________________
Please keep all replies on the list by using "reply all"
in your mail client.  To manage your subscriptions to this
and other Galaxy lists, please use the interface at:
  http://lists.bx.psu.edu/

To search Galaxy mailing lists use the unified search at:
  http://galaxyproject.org/search/mailinglists/