Attachment 68667 Details for Bug 17989 – Bug 17989: Centralize bad template check

[patch] Bug 17989: Centralize bad template check

Bug-17989-Centralize-bad-template-check.patch (text/plain), 3.64 KB, created by Nick Clemens (kidclamp) on 2017-10-27 09:41:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2017-10-27 09:41:29 UTC

Size: 3.64 KB

patch

obsolete

>From f0f54f45843b47b288c5a5a398d446370c94016f Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Mon, 30 Jan 2017 08:43:45 +0100
>Subject: [PATCH] Bug 17989: Centralize bad template check
>
>The bad template check in get_template_and_user can be removed, since
>this check has been added in gettemplate on bug 18010.
>
>The check moves here to a new sub in C4::Templates. And will be extended
>in a follow-up.
>
>Removed unused variable $path in gettemplatefile.
>
>Test plan:
>[1] Run t/db_dependent/Auth.t (get_template_and_user bad calls).
>[2] Run t/db_dependent/Templates.t (bad call to gettemplate).
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> C4/Auth.pm         |  5 ++---
> C4/Templates.pm    | 20 ++++++++++++++++++--
> Koha/Exceptions.pm |  4 ++++
> 3 files changed, 24 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index 11f57b2..f2b5f6c 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -161,10 +161,9 @@ sub get_template_and_user {
> 
>     C4::Context->interface( $in->{type} );
> 
>-    my $safe_chars = 'a-zA-Z0-9_\-\/';
>-    die "bad template path" unless $in->{'template_name'} =~ m/^[$safe_chars]+\.tt$/ig; #sanitize input
>-
>     $in->{'authnotrequired'} ||= 0;
>+
>+    # the following call includes a bad template check; might croak
>     my $template = C4::Templates::gettemplate(
>         $in->{'template_name'},
>         $in->{'type'},
>diff --git a/C4/Templates.pm b/C4/Templates.pm
>index 17746e8..94c5baf 100644
>--- a/C4/Templates.pm
>+++ b/C4/Templates.pm
>@@ -37,6 +37,7 @@ use C4::Languages qw(getTranslatedLanguages get_bidi regex_lang_subtags language
> use C4::Context;
> 
> use Koha::Cache::Memory::Lite;
>+use Koha::Exceptions;
> 
> __PACKAGE__->mk_accessors(qw( theme activethemes preferredtheme lang filename htdocs interface vars));
> 
>@@ -167,15 +168,30 @@ sub _get_template_file {
>     return ($htdocs, $theme, $lang, $filename);
> }
> 
>+=head2 badtemplatecheck
>+
>+    badtemplatecheck( $template_path );
>+
>+    The sub will throw an exception if the template path is not allowed.
>+
>+    Note: At this moment the sub is actually a helper routine for
>+    sub gettemplate.
>+
>+=cut
>+
>+sub badtemplatecheck {
>+    my ( $template ) = @_;
>+    Koha::Exceptions::NoPermission->throw( 'bad template path' )
>+        unless $template =~ m/^[a-zA-Z0-9_\-\/]+\.(tt|pref)$/;
>+}
> 
> sub gettemplate {
>     my ( $tmplbase, $interface, $query, $is_plugin ) = @_;
>     ($query) or warn "no query in gettemplate";
>-    die "bad template path" unless $tmplbase =~ m/^[a-zA-Z0-9_\-\/]+\.(tt|pref)$/; # Will be extended on bug 17989
>-    my $path = C4::Context->preference('intranet_includes') || 'includes';
>     my ($htdocs, $theme, $lang, $filename)
>        =  _get_template_file($tmplbase, $interface, $query);
>     $filename = $tmplbase if ( $is_plugin );
>+    badtemplatecheck( $filename ); # single trip for bad templates
>     my $template = C4::Templates->new($interface, $filename, $tmplbase, $query);
> 
> # NOTE: Commenting these out rather than deleting them so that those who need
>diff --git a/Koha/Exceptions.pm b/Koha/Exceptions.pm
>index 528a151..9d7f397 100644
>--- a/Koha/Exceptions.pm
>+++ b/Koha/Exceptions.pm
>@@ -33,6 +33,10 @@ use Exception::Class (
>         isa => 'Koha::Exceptions::Exception',
>         description => 'One or more parameters are wrong',
>     },
>+    'Koha::Exceptions::NoPermission' => {
>+        isa => 'Koha::Exceptions::Exception',
>+        description => 'You do not have permission for this action',
>+    },
>     'Koha::Exceptions::CannotAddLibraryLimit' => {
>         isa => 'Koha::Exceptions::Exception',
>         description => 'General problem adding a library limit'
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17989: 65277 | 65278 | 65279 | 65280 | 65281 | 66272 | 66273 | 66274 | 66275 | 66276 | 66317 | 66318 | 66319 | 66320 | 66321 | 67514 | 67515 | 67516 | 67517 | 67518 | 68654 | 68667 | 68668 | 68669 | 68670 | 68671 | 68672 | 68842 | 68843