MARC Editor XSS vulnerability

This also affects the $y and maybe other MARC fields that render to HTML

Branch user/jeffdavis/lp1902965-marc-856-filter in the security repo fixes the issue by applying the TT2 "html" filter to the URI href, link, and note before using them. I'm not sure if escaping HTML entities will cause problems for certain 856 URLs.

I haven't checked whether other MARC fields have the same vulnerability, but it seems likely.

Unfortunately it seems the demo.evergreencatalog.com I was testing on upgraded to the new OPAC since I reported this so the playing field has changed -- but on there, 245 and 100 fields are vulnerable there. Will try to test some more things

Here's some more fields I found that seem vulnerable:
010$a
245$abp

Also supercat HTML output is vulnerable (/opac/extras/supercat/retrieve/html/record/<id>)
260$c
260$b

Here's what I'm using to generate Evil MARC Records:
https://gist.github.com/jamesrf/1179f75ec51da878d0577a21575bffa5

In my testing, 245$abp are also vulnerable in search results (but not if Show More Details is enabled).

The following fields are vulnerable in record summary, in addition to 010$a and 245$abp:
250$a
260$abc
740$a

New security branch user/jeffdavis/lp1902965-filter-marc-fields has the following fixes:

- Filter a long list of MARC fields in OPAC search results and record display.
- Apply the same filters in the Bootstrap OPAC.
- Change format of rights element in MARC21slim2ATOM.xsl to avoid XSS in SuperCat record display.

Hi, I have a bootstrap fix for Jeff's branch at security/user/jboyer/lp1902965_marc_xss ... but I've been stuck trying to figure out how to correct the instance I've noticed in the 245a in the 3.6+ Angular staff catalog.

To see what I mean, edit a record to add <i>Yo, Italics!</i> to a 245 a, then load the record by id. You should see that message in italics in the Record Summary at the top of the page. If you instead *search* for the record (by title, author, whatever) and click it in the search results, you *will not* see the title italicized. It will instead show the literal <i> </i> tags in the summary as you would expect. I believe this is because Display Fields are used when performing searches and not when loading a record directly but I'm not having a lot of luck tracking down where the difference is in the client.

I think this is one of the few remaining situations like this in the client but this feels like a game of whack-a-mole that we're likely to eventually lose. I wonder if we may have to eventually alter the way things work at a lower level to really put an end to this.

The attached SQL file will insert a bad MARC record (Nine Hundred Grandmothers by R. A. Lafferty) into biblio.record_entry; after insertion, retrieving the bad record by ID should show a bunch of alert dialogs unless the fix is applied.

I have pushed my signoff of Jeff's and Jason's commits to security/collab/dyrcona/lp1902965_marc_xss.

I noticed that italics and emphasis tags did have an effect in the Angular Staff catalog view. However, script tags were unable to open alerts. I didn't explore this much farther, so that bears looking into. I would be content to consider that a different bug in order to get this into a release sooner.

I have pushed backport branches for rel_3_5 and rel_3_4 to the security repository:

collab/dyrcona/lp1902965_marc_xss-rel_3_5

and

collab/dyrcona/lp1902965_marc_xss-rel_3_4

I was planing to include this in this week's releases. The code back ports cleanly to 3.6, 3.5, and 3.4 if you skip the bootstrap commits for 3.5 and 3.4

However, the code appears to have a conflict with the read more/accordion code in master, and I do not feel competent to resolve it.

I've removed the release targets from the bug and added the needsrepatch tag.

Thanks for testing, Jason.

Security branch user/jeffdavis/lp1902965-filter-marc-fields-2 includes a revised version of the TPAC changes to accommodate the Read More accordion. (Please note I have force-pushed changes to this branch since it was first created a few days ago.) The previous branch should be used for 3.6 and earlier releases.

In this new branch, the accordion macro takes care of filtering, so any test that gets accordion'd shouldn't need an additional html filter.

It appears that the accordion does not exist in the Bootstrap OPAC, so there is some code drift happening between the OPACs. Some adjustments to HTML filtering will be necessary if and when the accordion does make it into Bootstrap.

It is frustrating to have to redo a security fix because it has languished while new features are committed.

I have not been able to reproduce the bug on master this morning.

I added a record that should trigger this bug to a concerto data set and added an item. I have not been able to find this record using the catalog search feature in order to open it with an affected interface.

I'm removing myself from this bug so someone else can have a look.

I've pushed to security/user/gmcharlt/lp1902965 a signoff of Jeff's latest along with a follow-up fixing an issue that it revealed in the accordianization of notes.

At this moment, I believe that the branches to base release-cutting:

3.7-beta: user/gmcharlt/lp1902965 (or more strictly speaking, cherry-picks from that)
3.4: security/collab/dyrcona/lp1902965_marc_xss-rel_3_4
3.5: security/collab/dyrcona/lp1902965_marc_xss-rel_3_5
3.6: security/collab/dyrcona/lp1902965_marc_xss-rel_3_6

Found an issue with this fix on 3.6. In the results table this mangles titles with the word "Hidden". Those titles end up invisible on the page.

Example: The hidden folk : stories of fairies, gnomes, selkies, and other hidden folk

So the word hidden is leaking and hiding the text.

<h3>
 <a class='record_title search_link' name='record_217866'
 href="/eg/opac/record/217866?query=folk;qtype=title;locg=1;detail_record_view=0"
 title=&quot;Display record details for &amp;quot;The hidden folk stories of fairies, gnomes,...&quot;>The hidden folk stories of fairies, gnomes, selkies, and other hidden folk </a>
</h3>

Hi, Bob!

Thanks for the report. Would you mind opening a new bug for your issue?

This one is "Fix Released" and your issue now warrants a new bug.