galkahana / HummusJS

Node.js module for high performance creation, modification and parsing of PDF files and streams
http://www.pdfhummus.com
Other
1.14k stars 169 forks source link

Invalid PDF input causes `SIGSEGV` crash #463

Closed mhassan1 closed 1 year ago

mhassan1 commented 1 year ago

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: https://github.com/julianhille/MuhammaraJS/issues/214

julianhille commented 1 year ago

Has been fixed in muhammara 3.1.1 and backported to 2.6.1

galkahana commented 1 year ago

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

julianhille commented 1 year ago

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

galkahana commented 1 year ago

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

galkahana commented 1 year ago

@julianhille im seeing just this guy: https://github.com/julianhille/MuhammaraJS/commit/1890fb555eaf171db79b73fdc3ea543bbd63c002

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 commented 1 year ago

Here are the ones i remember:

https://github.com/julianhille/MuhammaraJS/commit/0a6427eec82ef2978995e453de2dc0d6224dd46c https://github.com/julianhille/MuhammaraJS/commit/4df356bd28114ff2962e92be40e5e740d3fd6ceb https://github.com/julianhille/MuhammaraJS/commit/90b278d09f16062d93a4160ef0a54d449d739c51

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

julianhille commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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