gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42747: (QA follow-up) Parameterize query, dedupe, and log rejections 

The original fix only whitelisted image_name at the point it's saved
in edit-layout.pl, leaving the actual injection sink in create-pdf.pl
unparameterized. This adds defense-in-depth by fixing the query itself,
avoids a redundant duplicate call to get_all_image_names() (already
fetched earlier in the script as $image_names), and logs rejected
image_name values instead of silently discarding them so tampering
attempts leave an audit trail.

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 5b9e06dafc78c7097ce4090b52fc63390df490fc)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize 2026-07-07 14:35:47 +01:00 committed by Lucas Gass
parent facf9f7c8b
commit 3f91a7a0b0
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 14 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
patroncards/create-pdf.pl
View File

@ -202,7 +202,8 @@ CARD_ITEMS:
my $dbh = C4::Context->dbh();
$dbh->{LongReadLen} = 1000000; # allows us to read approx 1MB
$image_data = $dbh->selectrow_hashref(
"SELECT imagefile FROM creator_images WHERE image_name = \'$images->{$card_image}->{'data_source'}->[0]->{'image_name'}\'"
"SELECT imagefile FROM creator_images WHERE image_name = ?",
undef, $images->{$card_image}->{'data_source'}->[0]->{'image_name'}
);
warn sprintf( 'Database returned the following error: %s.', $error ) if $error;
unless ($image_data) {

23
patroncards/edit-layout.pl
View File

@ -34,6 +34,7 @@ use C4::Creators qw(
get_unit_values
);
use C4::Patroncards;
use Koha::Logger;
my $cgi = CGI->new;
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
@ -196,10 +197,8 @@ UNUSED_IMAGE_FIELDS:
my $image_select = 0;
my $field_enabled = 0;
# Get valid image names for validation
my $valid_image_names = get_all_image_names();
my %valid_images = map { $_->{'type'} => 1 } @$valid_image_names;
$valid_images{'none'} = 1; # 'none' is also a valid value
# $image_names was already fetched above (and already includes 'none')
my %valid_images = map { $_->{'type'} => 1 } @$image_names;
CGI_PARAMS:
@ -232,13 +231,15 @@ CGI_PARAMS:
my $param_value = $cgi->param($parameter);
# Sanitize image_name
my $field_name = $image_data =~ m/^image_(.*)$/ ? $1 : $image_data;
$param_value = undef
if $field_name eq 'name'
&& $param_value
&& !exists $valid_images{$param_value};
if ( $image_data =~ m/^image_(.*)$/ ) {
$layout->{'images'}->{"image_$image_number"}->{'data_source'}->{"image_$1"} = $param_value;
my ($image_suffix) = $image_data =~ m/^image_(.*)$/;
my $field_name = $image_suffix // $image_data;
if ( $field_name eq 'name' && $param_value && !exists $valid_images{$param_value} ) {
Koha::Logger->get->warn(
sprintf( "Rejecting unknown image_name '%s' submitted for patroncard layout", $param_value ) );
$param_value = undef;
}
if ( defined $image_suffix ) {
$layout->{'images'}->{"image_$image_number"}->{'data_source'}->{"image_$image_suffix"} = $param_value;
} else {
$layout->{'images'}->{"image_$image_number"}->{$image_data} = $param_value;
}