From cbc708667726437103ea28e8bb920586cec5d2e9 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Date: Tue, 7 Jul 2026 14:35:47 +0100
Subject: [PATCH] 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 5b825da072daf7367a538cb3cb8ecceb9b433271)
Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
---
 patroncards/create-pdf.pl  |  5 ++++-
 patroncards/edit-layout.pl | 23 ++++++++++++-----------
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/patroncards/create-pdf.pl b/patroncards/create-pdf.pl
index e994ddbbb2..61cdee5148 100755
--- a/patroncards/create-pdf.pl
+++ b/patroncards/create-pdf.pl
@@ -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'});
diff --git a/patroncards/edit-layout.pl b/patroncards/edit-layout.pl
index 1a73612ebd..f88d37ba17 100755
--- a/patroncards/edit-layout.pl
+++ b/patroncards/edit-layout.pl
@@ -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;
             }