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-devel@pmichaud.com http://www.pmichaud.com/mailman/listinfo/pmwiki-devel