[pmwiki-users] ZAP and htmlspecialchars...

[The Editor]

On 10/30/06, Patrick R. Michaud <pmichaud at pobox.com> wrote:
> On 10/22/06, The Editor <editor at fast.st> wrote:
> > On 10/22/06, Patrick R. Michaud <pmichaud at pobox.com> wrote:
> > > > 1) If I'm using PageUpdate for all file changes, do I need to worry
> > > > about using htmlspecialchars?  My assumptions is PmWiki takes care of
> > > > all that for me.
> > >
> > > That would be an incorrect assumption.  PmWiki's built in markups
> > > are okay, but if ZAP adds any markups that produce output from
> > > user-contributed source data, ZAP needs to be handling htmlspecialchars.
> >
> > I took a stab at it, but not sure I got them right. I wouldn't even
> > know what to do to test if it was working right!  : )  Do these look
> > ok?
> >
> > Markup('select', 'inline', '/\(:select (.*?):\\)/',
> >    "htmlspecialchars(\"<select name=$1>\")");
>
> Ah.  If you're grabbing data from existing markup (e.g.,
> from a capturing parentheses in the pattern of a markup rule),
> then htmlspecialchars isn't needed here, because PmWiki has
> already run htmlspecialchars on the markup text.
>
> Where htmlspecialchars() is generally needed is when getting
> data from another source *other* than the markup of the current page.
>
> So, eliminating htmlspecialchars from the above give us:
>
>    Markup('select', 'inline',
>      '/\\(:select (.*?):\\)/',
>      "<select name=$1>");
>
> However, this has some other problems.  First, all XHTML
> attributes should be quoted, thus name='$1' instead of name=$1.
> But more troubling is that the above invites a cross-site
> scripting vulnerability if anyone can edit the page, thus:
>
>    (:select xyz onclick='alert("Ooops!")':)
>
> produces
>
>    <select name=xyz onclick='alert("Ooops!")'>
>
> and that's not a good thing, since the author can get
> onclick= to do a number of undesirable operations.
>
> > Markup('textarea', 'inline', '/\\(:textarea (.*?):\\)/e',
> >    "Keep(PSS(htmlspecialchars(\"<textarea $1>\")))");
> > Markup('option', 'inline', '/\\(:option (.*?):\\)/e',
> >   "Keep(PSS(\"<option value='$1'>\"))");
>
> These also have the XSS vulnerability (and don't need
> htmlspecialchars, because they're drawing from markup of
> the current page).

Thanks for looking at these, very much!  In this case, it may not be a
problem because I'm strongly recommending no admin allow open editing
of pages with ZAP forms.  If fact, this might be good so you could
embed something like onchange=document.form.submit() in a pull down
menu.  What do you think?

As for the textareas, if the only vulnerability is in the markup, not
the data that's typed in, there's probably no need to fix it--as only
admins should be using these.  And one day these will be replaced by
your new forms script.  But if it should be closed, how do you do it?
Create a function that does a string replace or something--and wrap it
into the markup?

Thanks again Patrick!

Cheers,
Caveman