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

Infinite loop if I decode a abnormal arraybuffer #13

Closed
Aqours opened this issue Nov 18, 2017 · 12 comments
Closed

Infinite loop if I decode a abnormal arraybuffer #13

Aqours opened this issue Nov 18, 2017 · 12 comments

Comments

@Aqours
Copy link

Aqours commented Nov 18, 2017

UPNG.js/UPNG.js

Lines 350 to 387 in c62ddc8

while(true) {
used=[]; plte=[];
var msk = 0xff - ((1<<pr)-1), add = ((1<<pr))>>1;
for(var i=0; i<area; i++) { var qi=i<<2; var r=nimg[qi],g=nimg[qi+1],b=nimg[qi+2],a=nimg[qi+3];
var nr=(r&msk)+add, ng=(g&msk)+add, nb=(b&msk)+add, na=(a&msk)+add, key=(na<<24)|(nb<<16)|(ng<<8)|nr;
if(used[key]) { var pv=plte[used[key]]; pv.occ++; }
else { used[key]=plte.length; plte.push( {occ:1, r:nr,g:ng,b:nb,a:na} ); }
if(plte.length>plim) break;
}
if(plte.length>plim) { pr++; continue; }
break;
}
if(pr==0 && plte.length<=ps) return bufs;
plte.sort(function(a,b) {return b.occ-a.occ;});
ps = Math.min(ps, plte.length);
var nplte = new Uint8Array(ps*4);
for(var i=0; i<ps; i++) { var qi=i<<2,c=plte[i]; nplte[qi]=c.r; nplte[qi+1]=c.g; nplte[qi+2]=c.b; nplte[qi+3]=c.a; }
plte = nplte; //*/
var icnt = Math.max(1, Math.min(10, Math.floor(1024/ps)));
var pind = new Uint16Array(area);
for(var it=0; it<icnt; it++)
{
UPNG.quantize.kMeans(nimg, plte, pind, ps, true);
}
UPNG.quantize.applyPalette(nimg, plte, pind);
// remap remaining frames according to palette
for(var i=1; i<imgs.length; i++) {
UPNG.quantize.kMeans(imgs[i], plte, pind, ps, false); // "classify" into pind
UPNG.quantize.applyPalette(imgs[i], plte, pind); // remap according to pind
}
//UPNG.quantize.dither(nimg, w,h, pind,plte, ps); // I think that (current) dithering is not worth it
for(var i=0; i<bufs.length; i++) imgs[i] = imgs[i].buffer;
return imgs;
}

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
Copy link
Owner

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

@Aqours
Copy link
Author

Aqours commented Nov 18, 2017

oh, I copy wrong line.

UPNG.js/UPNG.js

Line 480 in c62ddc8

while(true)

Abnormal arraybuffer obtained by FileReader API.

@photopea
Copy link
Owner

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

@photopea
Copy link
Owner

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
Copy link
Author

Aqours commented Nov 18, 2017

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
Copy link
Owner

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
Copy link
Author

Aqours commented Nov 21, 2017

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
Copy link
Owner

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

@Aqours
Copy link
Author

Aqours commented Nov 22, 2017

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
Copy link
Owner

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
Copy link
Author

Aqours commented Nov 23, 2017

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
Copy link
Owner

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

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

2 participants