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

Invalid PDF input causes SIGSEGV crash #463

Closed
mhassan1 opened this issue Oct 26, 2022 · 15 comments
Closed

Invalid PDF input causes SIGSEGV crash #463

mhassan1 opened this issue Oct 26, 2022 · 15 comments

Comments

@mhassan1
Copy link

Certain invalid PDFs cause the Node.js process to crash with SIGSEGV. I can provide an example.

Even though it is an invalid PDF, it should not cause the Node.js process to crash with SIGSEGV, which can't be caught; instead, it should throw an Error, like it does for other invalid PDFs.

There is a duplicate issue in muhammara, a fork of hummus: julianhille/MuhammaraJS#214

@julianhille
Copy link
Contributor

Has been fixed in muhammara 3.1.1 and backported to 2.6.1

@galkahana
Copy link
Owner

thanks. i'll take care of this too. both in the C++ version and JS. thanks @mhassan1 and @julianhille.

@julianhille
Copy link
Contributor

There are at least 3 of these npe exceptions in the writer. Should I send you some patches?

@galkahana
Copy link
Owner

i don't mind scanning your latest corrections, it's fine. thanks.

@galkahana
Copy link
Owner

@julianhille im seeing just this guy:
julianhille/MuhammaraJS@1890fb5

there's also this:
https://github.com/julianhille/MuhammaraJS/pull/189/files#diff-41948bad0bb698c73e4bf20e262152057149ec83c6c534c6683e3cc66b74b666
which is limited to the js driver...guess i can add a correction for that too.

not sure i find other matters. if you can point me to them, good.

@julianhille
Copy link
Contributor

Here are the ones i remember:

julianhille/MuhammaraJS@0a6427e
julianhille/MuhammaraJS@4df356b
julianhille/MuhammaraJS@90b278d

Side question, cause you are here: :)
Will you still provide security updates to hummus?
So should i mention you or create security related tickets here and maybe PRs?
Same question for PDF Writer, cause it has been a long time without maintenance so i thought its abandoned / discontinued.

If not it would be awesome to point people to muhammara, at least in case of hummus, "merge" community and keep improving.

@galkahana
Copy link
Owner

galkahana commented Nov 2, 2022

I'll provide them as long as my interests in using hummus remains. as it happens im using it now and this comes handy. going forward people should use muhammara for guarantee of service. still...there's still quite a lot of people using hummusjs and pdfwriter now. i don't mind if they benefit from this, so i'll publish.

@julianhille
Copy link
Contributor

Btw my actual plan was to fork, disconnect the fork, and keep maintaining pdf writer, then use it as a submodule for muhammara. That would mean all sides would benefit from updates / improvements and it would be less duplicated PRs / commits / work.

If you would like we could have a chat about it.

@galkahana
Copy link
Owner

yeah but then there's those nifty features added that i don't want. so no...muhammara will be the people's wishes, and hummus will fulfil my own. sort of an old school opensource when it was about sharing sources and not endless mundane maintainance. hopefully people will stop using it sometime. though it doesn't seem to be that way still, unfortunately.

@galkahana
Copy link
Owner

i'll provide notice in npm and github of hummusjs for people to go to muhammara

@julianhille
Copy link
Contributor

Ok, great. so i'll keep you posted on important security updates. Should i mention you, mail you or just open a PR / issue (what ever comes in handy) in the PDF writer lib?

@galkahana
Copy link
Owner

pff. i guess mentioning would be best to get my attention. but man don't be too disappointed if im not responding, i guess im not a great guy ;). and thanks for your work. it's a good parser/writer and shame it'd go to shit just cause i'm bummed down by everything.

anyways, let's also connect on linkedin while at it, if that makes sense to you:
https://www.linkedin.com/in/galkahana/
it's another com channel i guess.

@julianhille
Copy link
Contributor

pff. i guess mentioning would be best to get my attention. but man don't be too disappointed if im not responding, i guess im not a great guy ;).

fair enough, whatever suits you :> (naaaw just other priorities)

it's a good parser/writer and shame it'd go to shit just cause i'm bummed down by everything.

yes it is a good writer. Things change and so do priorities, no problem.

will contact you later.

anyways, let's also connect on linkedin while at it, if that makes sense to you: https://www.linkedin.com/in/galkahana/ it's another com channel i guess.

@galkahana
Copy link
Owner

ok updated pdfwriter and hummusjs. version 1.0.111 should contain the 3 corrections. there's still some binaries to load for mac (ran out of travis credits) and i didn't take care of node18 et i guess. but at least the code is right.
also included link to yr site.

@galkahana
Copy link
Owner

Folks. Gonna close this. If theres any more requests lemme know/reopen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants