Re: strange conversions: a FmtPageName bug

Sunday, March 9, 2008, 2:33:29 AM, Patrick wrote:

> Also, as a general rule it's unwise to be calling
FmtPageName()
> on strings that are coming from page markup, as this
exposes
> the ability for people to view the values of variables
that
> perhaps they shouldn't see.  This is also why page
variables
> (which come from markup) use PageVar() and
PageTextVar() and
> don't go through FmtPageName().

Sorry about the late entry into this discussion...  I just
joined
pmwiki-devel and was looking over the last couple months
archives to see
what kind of discussions had been going on.  This one JUMPED
out at me...

I had thought from various pmwiki-users posts as well as
from the
documentation at http://
www.pmwiki.org/wiki/PmWiki/FmtPageName that this was
a function designed to take all the work out of
substitutions in recipe
development.  HOWEVER, according to this post (above) we
shouldn't be using
it on any user-supplied text.  OOPS!!!  98% of the text on
which recipe
developers (or at least I) want to do substitutions is on
user-supplied text
(specifically page markup).  

(1) This security issue REALLY needs to be documented on
http://
www.pmwiki.org/wiki/PmWiki/FmtPageName!!!  I just read
it through
again and unless I'm missing something there's no mention of
any security
risk...  I'll try to take a few minutes in the next few days
to put
something out there, but probably it should come from
someone who
understands the issue better...?

(2) Is there any possibility (please, please, please!) that
FmtPageName()
could be divided into 2 pieces in a backwards compatible way
in core?
Here's what I've got in mind - something that should (as far
as I can see)
have NO impact whatsoever on existing programs but would
give recipes a
"user-supplied-text-safe" hook to do text
substitution...

===CURRENT FMTPAGENAME DEFINITION:===
function FmtPageName()
{
   1. Replace any sequences of the form $XyzFmt with value
of any
corresponding global variable.
   2. Process the string for any $[...] phrases
(internationalized phrase),
using the currently loaded translation tables.
   3. Perform any pattern replacements from the array $FmtP.
Typically this
is used to handle things like $Name and $Group etc that are
specific to the
name of the current page.
   4. If $EnablePathInfo isn't set, convert URIs to use the
syntax
$ScriptUrl?n=<Group>.<Name> instead of
$ScriptUrl/<Group>/<Name>.
   5. Replace any $-sequences with global variables (caching
as needed) of
the same name (in reverse alphabetical order) *
   6. Replace any $-sequences with values out of the array
$FmtV. 
}
===(snip)===

===PROPOSED FMTPAGENAME DEFINITION:===
function FmtPageName()
{
   FmtPageNameSafe()
   5. Replace any $-sequences with global variables (caching
as needed) of
the same name (in reverse alphabetical order) *
   6. Replace any $-sequences with values out of the array
$FmtV. 
}

Function FmtPageNameSafe()
{
   1. Replace any sequences of the form $XyzFmt with value
of any
corresponding global variable.
   2. Process the string for any $[...] phrases
(internationalized phrase),
using the currently loaded translation tables.
   3. Perform any pattern replacements from the array $FmtP.
Typically this
is used to handle things like $Name and $Group etc that are
specific to the
name of the current page.
   4. If $EnablePathInfo isn't set, convert URIs to use the
syntax
$ScriptUrl?n=<Group>.<Name> instead of
$ScriptUrl/<Group>/<Name>.
}
===(snip)===

This gives recipe authors a hook to call (FmtPageNameSafe()
- although the
name needs to be thought through more effectively) which
they can use to do
substitutions on user-supplied text while still leaving
FmtPageName() to be
identical with its current operation.

Am I right in assuming it's the global variable substitution
that introduces
the security risk?  Or are some of the other substitutions
also potentially
risky from a security standpoint?

I, too, have been greatly frustrated by the substitution
occurring on short
globals ($p is my nightmare in various contexts) -- I would
*love* to be
able to do my substitution without using the globals at all
and presumably
this would also give me a "safe" function to use
from a security
perspective.

Any possibilities?  Or is PITS (and voting in that context)
the way
something like this moves ahead?

-Peter

PS Another alternative would be to supply an optional
parameter to "bail
out" early if someone doesn't want those final 2 steps
(related to global
variables) to be substituted...  Pseudo code below:

===ALTERNATE FMTPAGENAME DEFINITION:===
function FmtPageName($fmt, $pagename, $safe_for_user_text =
false)
{
   1. Replace any sequences of the form $XyzFmt with value
of any
corresponding global variable.
   2. Process the string for any $[...] phrases
(internationalized phrase),
using the currently loaded translation tables.
   3. Perform any pattern replacements from the array $FmtP.
Typically this
is used to handle things like $Name and $Group etc that are
specific to the
name of the current page.
   4. If $EnablePathInfo isn't set, convert URIs to use the
syntax
$ScriptUrl?n=<Group>.<Name> instead of
$ScriptUrl/<Group>/<Name>.

	if ($safe_for_user_text) return($fmt);

   5. Replace any $-sequences with global variables (caching
as needed) of
the same name (in reverse alphabetical order) *
   6. Replace any $-sequences with values out of the array
$FmtV. 
}
===(snip)===

With this solution you could also if-out earlier steps if
they were deemed
unsafe...


_______________________________________________
pmwiki-devel mailing list
pmwiki-develpmichaud.com
http://www.pmichaud.com/mailman/listinfo/pmwiki-devel