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: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-07-07 14:35:47 +01:00 committed by Pedro Amorim
parent bbbaa83c1c
commit 5aa86845f5
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
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(); my $dbh = C4::Context->dbh();
$dbh->{LongReadLen} = 1000000; # allows us to read approx 1MB $dbh->{LongReadLen} = 1000000; # allows us to read approx 1MB
$image_data = $dbh->selectrow_hashref( $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; warn sprintf( 'Database returned the following error: %s.', $error ) if $error;
unless ($image_data) { unless ($image_data) {

23
patroncards/edit-layout.pl
View File

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