[BioRuby] Bioruby HTML output

[Pjotr Prins]

On Tue, Jan 12, 2010 at 06:29:57PM +0900, Naohisa GOTO wrote:
> I'm not sure whether the prefix Bio::Html is suitable or not.

Me neither ;). This is something to discuss when we meet. See my
write up on partitioning based on functionality or standards.

> By the way, I'v tried some of your code in
> http://github.com/pjotrp/bioruby/blob/color-alignment/
> and found potential XSS.
> 
>   a = Bio::Alignment.new
>   a.add_seq('ATCCATGG', '<script>alert("a");</script>')
>   a.add_seq('ATGCATGC', '<script>alert("b");</script>')
>   a.add_seq('<script>alert("c");</script>', 'c')
>   simple = Bio::Html::HtmlAlignment.new(a,
>           :title => '<script>alert("title");</script>')
>   html = simple.html()
>   File.open('/tmp/xss.html', 'w') { |w| w.print html }
> 
> For sequences, sequence names, and consensus lines,
> using CGI.escapeHTML() will always be needed.
>
> For the :title, if script users can set the title, it
> should be escaped, but this prevents script programmers
> using html tags in the title.

Perhaps the HTML generator should escape its output. Though I
personally think we should only be worried about security concerns
when people *enter* new data on input forms. That is when exploits
show up. I can argue that HTML generation should not concern itself
with HOW the inputs are presented. One advantage of having a
programmer set the 'title' is that he *can* embed HTML. Perhaps
escaping HTML is the responsibility of the programmer providing the
data. And therefore to the logic that handles input.

We have had a similar discussion before. We have to decide to what
level *output* code should concern itself with *input* security. I
have a feeling that too much of Bioruby classes try to do too much.
How do we stay away from cluttering the code? How do we decide that
callers should not use HTML and handle security concerns?

You write:

>   a.add_seq('ATCCATGG', '<script>alert("a");</script>')

If a programmer wants that - it is his concern in my opion. If he is
concerned about exploits he should not allow it. The Alignment class
does not care either. It is none of its business.

BTW I fixed a number of PAML::Codeml bugs on this branch. So you
can ignore the existing PAML branch. Let's continue with the color
coding, assuming you can live with the PAML::Codeml implementation,
as it stands.

Pj.