Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pdf.js CVE not detected due to fork #2719

Open
spocky opened this issue Jan 9, 2025 · 4 comments
Open

pdf.js CVE not detected due to fork #2719

spocky opened this issue Jan 9, 2025 · 4 comments
Assignees
Labels
Loving it! Sometimes I have to decline a feature I'd love to implement. Let's give these features some love! Stephan wants to look at it!

Comments

@spocky
Copy link

spocky commented Jan 9, 2025

Sorry, not sure how to tag this.
We recently ran into a major issue : a pentest showed that the ngx-extended-pdf-viewer we were using (19.2.1) was vulnerable to CVE-2024-4367. This has been fixed on your side in 20.0.2.

I believe there are 2 issues regarding this :

  • I don't think 19.x has been fixed (that could have been automatically updated during project build on our side thanks to npm)
  • this project is using a fork of pdf.js instead of a direct dependency. While I totally understand the reason, it made npm audit unaware of the CVE in our project

To make is less problematic if (when) the next CVE on pdf.js arises, maybe we can :

  • publish a bugfix version on all the recent (?) ngx-extended-pdf-viewer instead of just the last one ? (time consuming, I suppose)
  • create a related CVE for ngx-extended-pdf-viewer (not sure how other projects using forks handle this) ?

What are your thoughts about this ?

@stephanrauh
Copy link
Owner

First of all, I'm sorry you ran into trouble!

Yes, I remember the CVE problem. It occurred during my vacations, when I had very little time to solve it. I managed to publish the bug fix quickly nonetheless. However, I did not find out how to report the CVE. I tried to do during my vacations, and I tried again after the vacations when I had more time, but I failed.

I think that's the most pressing issue, and maybe you can help me with that. How can I report a CVE? I'd be happy if npm knew that version 19.2.x is bogus. As far as I remember, I reported the issue to GitHub, and they closed the issue because it has been solved in Mozilla's branch. Any ideas?

As for publishing the bug fix in old versions: well, I agree I should do it, but it's not that easy. I'm ill prepared. Starting with version 22.2.x, I'm tagging the fork of pdf.js properly, so I hope I can publish back-ports to old versions in future. But even so, that's something I avoid because it's an error-prone process. I almost always mess it up. But, then, a CVE is a good reason to do it nonetheless!

Unfortunately, that doesn't help us with version 19. I'm afraid it's next to impossible to publish the bugfix.

BTW, this kind of problems is why I've published the library under an Apache V2 license. It means you're using it at your own risk. I tend to fix bugs quickly, but there's no warranty. And with very few exceptions, I only fix bugs in the latest version, and I say so frequently. But that's only the legal point of view. I intend to be better, it's just that I can't promise anything.

@stephanrauh stephanrauh self-assigned this Jan 9, 2025
@stephanrauh stephanrauh added Stephan wants to look at it! Loving it! Sometimes I have to decline a feature I'd love to implement. Let's give these features some love! labels Jan 9, 2025
@spocky
Copy link
Author

spocky commented Jan 9, 2025

Thanks for your detailed response!

To be completely transparent (not sure this was clear in my 1st message), I'm not blaming you for not publishing bugfixes for older versions. I totally understand what it takes to maintain a project like this one.
I'm just arising this because although CVEs are pretty rare, they happen sometimes, and being able to detect them early in a project is important to keep clients trust.
Regarding this specific issue, we've fixed it by updating to 20.0.2. I'm not sure how we could have detected it earlier on our side, though (except by looking at your changelog, but we usually only update project dependencies every few months).
Things could probably be improved, on our side and on ngx-extended-pdf-viewer's. This is an open discussion. Maybe we can add the official pdf.js as a dev-dependency in our project, just so that npm audit informs us of future CVEs, while not cluttering the app build ?

As to how declare a CVE, I'm as clueless as you. I never did it before. Not even sure a project containing a fork should get its own CVE. Maybe somebody reading this will be more knowledgeable ?

@stephanrauh
Copy link
Owner

stephanrauh commented Jan 10, 2025

Don't worry, your message didn't feel like blaming. I decided to apologize because it felt like the right thing to do. I want you to enjoy using my library - and that didn't work in your case, at least not during the pen test.

(On the other hand, I'm glad you ran the pen test, and I'd like to congratulate the pen test team. You did good work, highly appreciated!)

I'm glad you've raised the issue. It's a loose end, and I've love to tie it up. But of course, I totally forgot about it in my daily business.

@stephanrauh
Copy link
Owner

By the way, if you're at liberty to update to the latest version, please do so. I believe version 22 is a major improvement over version 20.0.2. Plus, I only fix bugs in the latest version, as mentioned above. I'm afraid that's also true for bugs. :) That said, my gut feeling is that versions 22.1.x and 22.2.x (currently alpha) are versions with a decent quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Loving it! Sometimes I have to decline a feature I'd love to implement. Let's give these features some love! Stephan wants to look at it!
Projects
None yet
Development

No branches or pull requests

2 participants