doc/bugs/404_when_cancel_create_page.mdwn

   1 If you
   2
   3  * Add a link to a non-existant page and save. (e.g. [[somewhere-over-the-rainbow]])
   4  * Click the question mark to create the page.
   5  * Click the cancel button.
   6
   7 You get a 404 as the page doesn't exist. This patch redirects to the from location
   8 if it is known.
   9
  10
  11         === modified file 'IkiWiki/CGI.pm'
  12         --- IkiWiki/CGI.pm
  13         +++ IkiWiki/CGI.pm
  14         @@ -427,7 +427,11 @@
  15                 }
  16
  17                 if ($form->submitted eq "Cancel") {
  18         -               redirect($q, "$config{url}/".htmlpage($page));
  19         +               if ( $newpage && defined $from ) {
  20         +                       redirect($q, "$config{url}/".htmlpage($from));
  21         +               } else {
  22         +                       redirect($q, "$config{url}/".htmlpage($page));
  23         +               }
  24                         return;
  25                 }
  26                 elsif ($form->submitted eq "Preview") {
  27
  28 > I think you mean to use `$newfile`? I've applied a modieid version
  29 > that also deal with creating a new page with no defined $from location.
  30 > [[bugs/done]] --[[Joey]]
  31
  32 [P.S. just above that is
  33
  34                 $type=$form->param('type');
  35                 if (defined $type && length $type && $hooks{htmlize}{$type}) {
  36                         $type=possibly_foolish_untaint($type);
  37                 }
  38                 ....
  39                 $file=$page.".".$type;
  40
  41 I'm a little worried by the `possibly_foolish_untaint` (good name for it by the way,
  42 makes it stick out). I don't think much can be done to exploit this (if anything),
  43 but it seems like you could have a very strict regex there rather than the untaint,
  44 is there aren't going to be many possible extensions. Something like `/(.\w+)+/`
  45 (groups of dot separated alpha-num chars if my perl-foo isn't failing me). You could
  46 at least exclude `/` and `..`. I'm happy to turn this in to a patch if you agree.]
  47
  48 > The reason it's safe to use possibly_foolish_untaint here is because
  49 > of the check for $hooks{htmlize}{$type}. This limits it to types
  50 > that have a registered htmlize hook (mdwn, etc), and not whatever random
  51 > garbage an attacker might try to put in. If it wasn't for that check,
  52 > using possibly_foolish_untaint there would be _very_ foolish indeed..
  53 > --[[Joey]]