Skip to content

Fix Glide GIF decoder vulnerability by validating logical screen dimensions. #5693

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.

Sign up for GitHub

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

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Glide GIF decoder vulnerability by validating logical screen dimensions. #5693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/StandardGifDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ public class StandardGifDecoder implements GifDecoder {
@ColorInt
private static final int COLOR_TRANSPARENT_BLACK = 0x00000000;

/** Maximum logical screen dimension a single GIF frame may declare (8K). */
private static final int MAX_VALID_DIMENSION = 8192;
/** Maximum total pixel count per frame (8K x 8K). */
private static final long MAX_VALID_PIXEL_COUNT = (long) MAX_VALID_DIMENSION * MAX_VALID_DIMENSION;

// Global File Header values and parsing flags.
/**
* Active color table.
Expand Down Expand Up @@ -359,6 +364,17 @@ public synchronized void setData(@NonNull GifHeader header, @NonNull ByteBuffer
if (sampleSize <= 0) {
throw new IllegalArgumentException("Sample size must be >=0, not: " + sampleSize);
}
if (header.width <= 0 || header.height <= 0
|| header.width > MAX_VALID_DIMENSION
|| header.height > MAX_VALID_DIMENSION) {
this.status = STATUS_FORMAT_ERROR;
return;
}
long pixelCount = (long) header.width * (long) header.height;
if (pixelCount > MAX_VALID_PIXEL_COUNT) {
this.status = STATUS_FORMAT_ERROR;
return;
}
// Make sure sample size is a power of 2.
sampleSize = Integer.highestOneBit(sampleSize);
this.status = STATUS_OK;
Expand All @@ -383,7 +399,7 @@ public synchronized void setData(@NonNull GifHeader header, @NonNull ByteBuffer
downsampledHeight = header.height / sampleSize;
// Now that we know the size, init scratch arrays.
// TODO Find a way to avoid this entirely or at least downsample it (either should be possible).
mainPixels = bitmapProvider.obtainByteArray(header.width * header.height);
mainPixels = bitmapProvider.obtainByteArray((int) pixelCount);
mainScratch = bitmapProvider.obtainIntArray(downsampledWidth * downsampledHeight);
}

Expand Down
56 changes: 54 additions & 2 deletions third_party/gif_decoder/src/test/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import androidx.annotation.NonNull;
import com.bumptech.glide.testutil.TestUtil;
import java.io.IOException;
import java.io.ByteArrayOutputStream;
import java.io.DataOutputStream;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -17,7 +19,7 @@

/** Tests for {@link com.bumptech.glide.gifdecoder.GifDecoder}. */
@RunWith(RobolectricTestRunner.class)
@Config(sdk = 19)
@Config
public class GifDecoderTest {

private MockProvider provider;
Expand Down Expand Up @@ -59,6 +61,8 @@ public void testCanDecodeFramesFromTestGif() throws IOException {
@Test
public void testFrameIndexStartsAtNegativeOne() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.frameCount = 4;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -69,6 +73,8 @@ public void testFrameIndexStartsAtNegativeOne() {
@Test
public void testTotalIterationCountIsOneIfNetscapeLoopCountDoesntExist() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.loopCount = GifHeader.NETSCAPE_LOOP_COUNT_DOES_NOT_EXIST;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -79,6 +85,8 @@ public void testTotalIterationCountIsOneIfNetscapeLoopCountDoesntExist() {
@Test
public void testTotalIterationCountIsForeverIfNetscapeLoopCountIsForever() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.loopCount = GifHeader.NETSCAPE_LOOP_COUNT_FOREVER;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -89,6 +97,8 @@ public void testTotalIterationCountIsForeverIfNetscapeLoopCountIsForever() {
@Test
public void testTotalIterationCountIsTwoIfNetscapeLoopCountIsOne() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.loopCount = 1;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -99,6 +109,8 @@ public void testTotalIterationCountIsTwoIfNetscapeLoopCountIsOne() {
@Test
public void testAdvanceIncrementsFrameIndex() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.frameCount = 4;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -110,6 +122,8 @@ public void testAdvanceIncrementsFrameIndex() {
@Test
public void testAdvanceWrapsIndexBackToZero() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.frameCount = 2;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand All @@ -123,6 +137,8 @@ public void testAdvanceWrapsIndexBackToZero() {
@Test
public void testSettingDataResetsFramePointer() {
GifHeader gifheader = new GifHeader();
gifheader.width = 1;
gifheader.height = 1;
gifheader.frameCount = 4;
byte[] data = new byte[0];
GifDecoder decoder = new StandardGifDecoder(provider);
Expand Down Expand Up @@ -170,13 +186,49 @@ public void testFirstFrameMustClearBeforeDrawingWhenLastFrameIsDisposalNone() th
assertTrue(firstFrame.sameAs(firstFrameTwice));
}

@Test
public void testDecodeOOMWithLargeGif() throws Exception {
byte[] data = buildMaliciousGif(30000, 30000);
GifHeaderParser headerParser = new GifHeaderParser();
headerParser.setData(data);
GifHeader header = headerParser.parseHeader();
GifDecoder decoder = new StandardGifDecoder(provider);
decoder.setData(header, data);
assertEquals(GifDecoder.STATUS_FORMAT_ERROR, decoder.getStatus());
}

@Test
public void testDecodeNegativeArraySizeWithNegativeDimensions() throws Exception {
byte[] data = buildMaliciousGif(32768, 32767);
GifHeaderParser headerParser = new GifHeaderParser();
headerParser.setData(data);
GifHeader header = headerParser.parseHeader();
GifDecoder decoder = new StandardGifDecoder(provider);
decoder.setData(header, data);
assertEquals(GifDecoder.STATUS_FORMAT_ERROR, decoder.getStatus());
}

private static byte[] buildMaliciousGif(int width, int height) throws Exception {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
DataOutputStream out = new DataOutputStream(baos);
out.writeBytes("GIF89a");
out.write(width & 0xFF); out.write((width >> 8) & 0xFF);
out.write(height & 0xFF); out.write((height >> 8) & 0xFF);
out.write(0xF7); // GCT=1, color depth=7, GCT size=7
out.write(0x00); out.write(0x00); // bg index, pixel aspect
for (int i = 0; i < 256; i++) { // 768-byte global color table
out.write(0x00); out.write(0x00); out.write(0x00);
}
out.write(0x3B); // GIF trailer
return baos.toByteArray();
}

private static class MockProvider implements GifDecoder.BitmapProvider {

@NonNull
@Override
public Bitmap obtain(int width, int height, Bitmap.Config config) {
Bitmap result = Bitmap.createBitmap(width, height, config);
Shadows.shadowOf(result).setMutable(true);
return result;
}

Expand Down
Loading