search

photopea / UPNG.js

Fast and advanced PNG (APNG) decoder and encoder (lossy / lossless)
MIT License
2.08k stars 255 forks source link

Infinite loop if I decode a abnormal arraybuffer #13

Closed Aqours closed 6 years ago

Aqours commented 6 years ago

https://github.com/photopea/UPNG.js/blob/c62ddc812f23abe629134e8a1527f48fe5dab2c1/UPNG.js#L350-L387

Above code would run infinite loop if I decode a abnormal arraybuffer, which block main JS thread, and will crash page. Consider using setTimeout or web worker, and I think it's better to apply decode timeout.

photopea commented 6 years ago

Hi, this function is only used for Encoding PNG, not for decoding. What kind of "abnormal" ArrayBuffer did you put into it?

Aqours commented 6 years ago

oh, I copy wrong line. https://github.com/photopea/UPNG.js/blob/c62ddc812f23abe629134e8a1527f48fe5dab2c1/UPNG.js#L480

Abnormal arraybuffer obtained by FileReader API.

photopea commented 6 years ago

How long is your ArrayBuffer? What makes it "abnormal" ? Can you send us your PNG file to support@photopea.com ?

photopea commented 6 years ago

The image, that you sent us, is a JPG image. This library can work only with PNG images. You should use some different library for parsing JPG images.

Aqours commented 6 years ago

I pay no attention to its size. Looks very small. And log “unknow chunk type”. You can download here https://github.com/Aqours/APNG-Codec/blob/master/test/elephant.png Look like iOS cannot download apng.

photopea commented 6 years ago

I tried to open your APNG with this version of UPNG.js and it works well.

Can you try to update your UPNG.js file? Are you sure you are sending an arraybuffer into decode(), not your FileReader or anything else?

Aqours commented 6 years ago

It works fine if using ArrayBuffer from 'correct' but not FileReader. ArrayBuffer from FileReader maybe not suitable for UPNG.decode method, but it should not decode infinite loop, because sometimes we don't know whether the ArrayBuffer is suitable for UPNG.decode method or not.

photopea commented 6 years ago

what do you mean by "correct"? whenever UPNG gets a valid PNG file, it should decode it in finite time.

Aqours commented 6 years ago

demo1 - http://git.hub.moe/laboratory/decode-apng-normal.html (open this link with modern browser) this demo works fine.

demo2 - http://git.hub.moe/laboratory/decode-apng-abnormal.html this demo does not work. you will see some message on browser's console. tim 20171122114605

photopea commented 6 years ago

In your picture, I see a message, that your PNG file was not found. This is not related to UPNG.js

The message from UPNG.js basically says, that you are not giving it a PNG file, but some other file (like PDF or EXE or something).

Aqours commented 6 years ago

I mean that unexpected arraybuffer should not cause UPNG.js blocking browser, because sometimes we don't kown whether the arraybuffer decoded by UPNG.js is excepted or not. (Inexistence png is just for demo) Throw an error or other ways might be better solution.

photopea commented 6 years ago

Ok, I added some basic PNG check. If the ArrayBuffer on the input is not a PNG file, it should throw an exception.