[pmwiki-devel] strange conversions: a FmtPageName bug

Peter & Melodye Bowers pbowers at pobox.com
Wed May 7 00:31:11 CDT 2008


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...




More information about the pmwiki-devel mailing list