From d4d937d2db29b349b489664079e28c51f4aa452c 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
Content-Type: text/plain; charset=utf-8

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>
---
 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 d22e935..5b9ec43 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 60b9e6a..ddc1275 100644
--- a/Koha/Exceptions.pm
+++ b/Koha/Exceptions.pm
@@ -28,6 +28,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