Bug 36799

Summary: Illegitimate modification of MARC authid field content (001)
Product: Koha Reporter: Janusz Kaczmarek <januszop>
Component: MARC Authority data supportAssignee: Janusz Kaczmarek <januszop>
Status: Pushed to oldstable --- QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, lucas, m.de.rooy
Version: unspecifiedKeywords: rel_24_05_candidate
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36794
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Attachments: Bug 36799: Illegitimate modification of MARC authid field content (001)
Bug 36799: Illegitimate modification of MARC authid field content (001)
Bug 36799: Illegitimate modification of MARC authid field content (001)

Description Janusz Kaczmarek 2024-05-07 09:09:15 UTC
It happens that librarians to save time open an authority editor by putting in the URL, by hand, the authid prefixed, by mistake, with a blank (e.g. .../authorities.pl?authid= 100 -- mind the space before 100).  In such a case the editor opens with the right auth record (i.e. 100) but, after saving the record, the content of the authid MARC field (001 for a standard MARC 21 installation) results modified and contains additional initial blanks.

Moreover, if the heading (1XX field) was modified in the authority record during such an edit, the changes will not propagate to the linked bibliographic records.  And won't in the future.

This is because in the authorities.pl script $authid is taken (and continuously used) directly from CGI parameter, without any validation and/or correction (line 540 in the current main branch).
Comment 1 Janusz Kaczmarek 2024-05-07 09:35:16 UTC
Created attachment 166245 [details] [review]
Bug 36799: Illegitimate modification of MARC authid field content (001)

It happens that librarians to save time open an authority editor by putting
in the URL, by hand, the authid prefixed, by mistake, with a blank (e.g.
.../authorities.pl?authid= 100 -- mind the space before 100).  In
such a case the editor opens with the right auth record (i.e. 100) but,
after saving the record, the content of the authid MARC field (001 for
a standard MARC 21 installation) results modified and contains
additional initial blanks.

Moreover, if the heading (1XX field) was modified in the authority record
during such an edit, the changes will not propagate to the linked
bibliographic records.  And won't in the future.

This is because in the authorities.pl script $authid is taken (and
continuously used) directly from CGI parameter, without any
validation and/or correction (line 540 in the current main branch).

Test plan:
==========
1. Open an auth record in the editor with an added space before
   authid value, e.g.:
   http://ktd:8081/cgi-bin/koha/authorities/detail.pl?authid= 100
   Modify the heading field -- in the ktd data set:
       150 Computerized typesetting
   Save the record.
2. a) Open the record for editing again--see the space added before
   the authid in field 001.  Close the editor (with Cancel).
   b) Try to go to the linked biblio records with Used in X records.
   Note no results, if using ES.
   c) Remove the space before authid (after an:) in the URL.
   Go to the linked biblio records.  See that the content of
   the field controlled by the modified auth record did not
   update.
3. Apply the patch; restart_all.
4. Repeat p. 1 and 2 with a different authid.  Everything should
   be OK now.

Sponsored-by: Ignatianum University in Cracow
Comment 2 Roman Dolny 2024-05-07 16:54:07 UTC
Created attachment 166315 [details] [review]
Bug 36799: Illegitimate modification of MARC authid field content (001)

It happens that librarians to save time open an authority editor by putting
in the URL, by hand, the authid prefixed, by mistake, with a blank (e.g.
.../authorities.pl?authid= 100 -- mind the space before 100).  In
such a case the editor opens with the right auth record (i.e. 100) but,
after saving the record, the content of the authid MARC field (001 for
a standard MARC 21 installation) results modified and contains
additional initial blanks.

Moreover, if the heading (1XX field) was modified in the authority record
during such an edit, the changes will not propagate to the linked
bibliographic records.  And won't in the future.

This is because in the authorities.pl script $authid is taken (and
continuously used) directly from CGI parameter, without any
validation and/or correction (line 540 in the current main branch).

Test plan:
==========
1. Open an auth record in the editor with an added space before
   authid value, e.g.:
   http://ktd:8081/cgi-bin/koha/authorities/detail.pl?authid= 100
   Modify the heading field -- in the ktd data set:
       150 Computerized typesetting
   Save the record.
2. a) Open the record for editing again--see the space added before
   the authid in field 001.  Close the editor (with Cancel).
   b) Try to go to the linked biblio records with Used in X records.
   Note no results, if using ES.
   c) Remove the space before authid (after an:) in the URL.
   Go to the linked biblio records.  See that the content of
   the field controlled by the modified auth record did not
   update.
3. Apply the patch; restart_all.
4. Repeat p. 1 and 2 with a different authid.  Everything should
   be OK now.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 3 Nick Clemens (kidclamp) 2024-05-10 14:17:30 UTC
Created attachment 166572 [details] [review]
Bug 36799: Illegitimate modification of MARC authid field content (001)

It happens that librarians to save time open an authority editor by putting
in the URL, by hand, the authid prefixed, by mistake, with a blank (e.g.
.../authorities.pl?authid= 100 -- mind the space before 100).  In
such a case the editor opens with the right auth record (i.e. 100) but,
after saving the record, the content of the authid MARC field (001 for
a standard MARC 21 installation) results modified and contains
additional initial blanks.

Moreover, if the heading (1XX field) was modified in the authority record
during such an edit, the changes will not propagate to the linked
bibliographic records.  And won't in the future.

This is because in the authorities.pl script $authid is taken (and
continuously used) directly from CGI parameter, without any
validation and/or correction (line 540 in the current main branch).

Test plan:
==========
1. Open an auth record in the editor with an added space before
   authid value, e.g.:
   http://ktd:8081/cgi-bin/koha/authorities/detail.pl?authid= 100
   Modify the heading field -- in the ktd data set:
       150 Computerized typesetting
   Save the record.
2. a) Open the record for editing again--see the space added before
   the authid in field 001.  Close the editor (with Cancel).
   b) Try to go to the linked biblio records with Used in X records.
   Note no results, if using ES.
   c) Remove the space before authid (after an:) in the URL.
   Go to the linked biblio records.  See that the content of
   the field controlled by the modified auth record did not
   update.
3. Apply the patch; restart_all.
4. Repeat p. 1 and 2 with a different authid.  Everything should
   be OK now.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Katrin Fischer 2024-05-10 15:40:59 UTC
As we have an expert for authorities/001 with Marcel, could you give your opinion? Is 001 being something other than numeric really impossible today?
Comment 5 Katrin Fischer 2024-05-10 15:45:46 UTC
Also somewhat blocked from doing tests here by Bug 36832 :(
Comment 6 Janusz Kaczmarek 2024-05-10 16:08:01 UTC
(In reply to Katrin Fischer from comment #4)
> As we have an expert for authorities/001 with Marcel, could you give your
> opinion? Is 001 being something other than numeric really impossible today?

Well, in fact, we are not talking here specifically about 001 which--in general--can contain anything.  We are talking about Koha authid stored--in standard Koha setup--in 001.  And Koha auth_header.authid is BIGINT UNSIGNED [20] (cf. https://schema.koha-community.org/main/tables/auth_header.html).  

Theoretically, one could store authid in a different place--the patch does nothing with 001, it does with authid.

Could you consider changing the status, please ;)
Comment 7 Katrin Fischer 2024-05-10 16:17:09 UTC
Not pushing anything more today - but as a bug fix we still got time for this. Also no string changes. Let me come back to this Monday.
Comment 8 Katrin Fischer 2024-05-13 08:42:31 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 9 Fridolin Somers 2024-05-29 12:05:26 UTC
Pushed to 23.11.x for 23.11.06
Comment 10 Lucas Gass 2024-05-30 19:18:35 UTC
Backported to 23.05.x for upcoming 23.05.12