PDF.js updates - Githubissues

sul-dlss / sul-embed

An oEmbed Service for Stanford University Libraries

Other

19 stars 6 forks source link

PDF.js updates #2157

Closed peetucket closed 2 weeks ago

peetucket commented 1 month ago

See https://github.com/sul-dlss/sul-embed/security/advisories/GHSA-3472-2v37-wmpj

jmartin-sul commented 1 month ago

Observations from planning:

setting isEvalSupported to false could lead to broken display for PDFs that expect to execute code (formatting macros? 🤷). do we have examples like this that we could test with?
the malicious PDF would have to be accessioned. people can't construct an embed link with an arbitrary malicious PDF that hasn't been deposited.
some amount of functionality regression is acceptable in this case, since we're aiming to fix a vulnerability (how much is acceptable: TBD).

jmartin-sul commented 1 month ago

may be superseded by #2162 -- after discussion just now in planning, we realized that we're closer to being able to use the native PDF viewer than we thought. the work for that viewer is merged, and controlled by a feature flag. we think the cutover to native PDF viewer will fine once the fullscreen problem of 2162 is taken care of, and a switch to the native PDF viewer would address the flaw we're trying to fix by configuring the isEvalSupported flag.

jmartin-sul commented 2 weeks ago

superseded by #2162. we decided to switch to the native PDF viewer instead of trying to upgrade PDFJS, or setting the isEvalSupported flag to false. the native PDF viewer gives a better experience anyway, and switching to it is less work than a PDFJS upgrade, and less disruptive to PDF viewing than setting isEvalSupported to false.