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>
(cherry picked from commit 5b825da072)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
This commit is contained in:
Martin Renvoize 2026-07-07 14:35:47 +01:00 committed by Baptiste Wojtkowski
parent 26418d381e
commit cbc7086677
No known key found for this signature in database
GPG Key ID: 47A8A13E10BC0D40
2 changed files with 16 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
patroncards/create-pdf.pl
View File

@ -177,7 +177,10 @@ foreach my $item (@{$items}) {
## FIXME: The DB stuff here needs to be religated to a Koha::Creator::Images object -chris_n
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'}\'");
$image_data = $dbh->selectrow_hashref(
"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){
warn sprintf('Image does not exists in db table %s.', $images->{$card_image}->{'data_source'}->[0]->{'image_name'});

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(
@ -179,10 +180,8 @@ elsif ($op eq 'cud-save') {
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:
foreach my $parameter ($cgi->multi_param()) { # parse the field values and build a hash of the layout for conversion to xml and storage in the db
@ -217,13 +216,15 @@ elsif ($op eq 'cud-save') {
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;
}