Forum OpenACS Development: Re: What a forking mess: noquote hell.

Collapse
Posted by Don Baccus on
Well ... the time to post arguments against something like the adoption of the noquote solution is when we're arguing over whether or not to undertake it - not months later.  Sorry, you're too late, I can't see us going back at this point so I don't see much point in rehasing the issue.  Of course if you can convince enough OCT members of the wisdom of backing it out, we'll do it, but frankly I don't think you'll succeed at this point.  The TIP process at least ensures in the future we'll have very visible decision points for major changes so everyone who cares will know to make their opinion public.  But it wasn't in place when noquote was adopted so we made the decision more informally ... but openly, following discussion in the forums.

Why not just backport the noquote changes into the 4.6.3 templating system and have it be a no-op for all but your package?  I don't think this would be very difficult ...

Collapse
Posted by Dirk Gomez on
What you want is sloppy and risky, only to ensure backwards compatibility you
want OpenACS to be insecure by default.

Please read http://www.cert.org/advisories/CA-2000-02.html

The first fix for "Web Page Developers and Web Site Administrators" is "
Web Page Developers Should Recode Dynamically Generated Pages to Validate
Output" and that is exactly what we did: by default html tags are now quoted
- this is the security aspect.

The more noquote tags in your adp scripts, the less security, you (may have)
captured that quite right.

I don't understand your example though: you created an html file outside of
OpenACS and loaded it and are now wondering about the successfull page
request? This is beyond the scope of the noquote patch. What you describe is
cross-site request forgery and for GET requests OpenACS isn't secured against
that, for post requests it is if you use the form builder.

Have a look here to see what changed in OpenACS code:

http://cvs.openacs.org/cvs/openacs-4/packages/acs-templating/tcl/parse-procs.tcl?r1=1.14&r2=1.15

Now if you adhere to your own high coding standards you can use the noquote
sed oneliners and maybe be getting away with only handful of manual changes.

Collapse
Posted by Dirk Gomez on
Forget about get and post requests. Use formbuilder and you are secured against cross-site request forgeries.

Where is the security risk in the particular request you posted Tom? Only if register were a (potentially) malicious request you had a security issue.

It is true that we need to add session tokens to a bunch of pages. See here for a post https://openacs.org/forums/message-view?message_id=32884

Collapse
Posted by Tom Jackson on

So if formbuilder protects against XSS, what extra problem does the noquote thing handle?

My example above just shows that you don't need scripting, you just need an image tag, did I really need to delete data or modify my website to prove that to you? Point is, the request is made with my privileges.

And you are absolutely right: changes can be made to form processing to handle the problem. So what was the noquote for again, especially since we are validating input?

Collapse
Posted by Jeff Davis on
You don't want to say "nohtml" for plain text fields since that means you can't use < in them. Take the forum subject field for example, you should be able to have a tag in the subject, like "<blink> should be on the list of allowed tags", but not have to worry that you will have a blinking page as a consequence. Of course you could ad_quotehtml all the plain text fields and not have noquote but given how few places this was done correctly in 4.6.3 and before I am confident that OpenACS with noquote has far fewer quoting bugs than previous versions.

Also, something like a user contribution page which shows data from lots of different packages (some of which might be missing validation like you are talking about) are much safer with automatic html quoting.

It certainly was not hard to move most of the toolkit to noquote and where it's wrong, the fix is pretty obvious so I think the lack of backward compatibility is not a big deal at all (and if you submit a patch versus 4.6.3 to make ;noquote a noop we would be happy to apply it).

Collapse
Posted by Tom Jackson on

It sounds like the noop is a good idea, I'll try it out and post a TIP, or bug/feature. I'm usually less than clear when I write. To summarize, noquote isn't a bad idea, there are other good reasons to use it, mentioned in the original documents referenced above. Upgrading my packages is a good idea, and I had already done one before I started to think, not about doing the upgrade, but about maintaining sites that can't move to 5.0 right away, but might want upgrades to my packages.

Collapse
Posted by Dirk Gomez on
Right we do both now. Filtering input has been there forever and filtering output has been added now.