From bb230e1b1993710e96f3d753bd61aad904f95ae8 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 30 Jan 2017 08:43:45 +0100 Subject: [PATCH] [SIGNED-OFF] 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 Signed-off-by: Josef Moravec --- 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