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

Use skipBytes(long) in places where integer overflow is possible #3529

Merged
merged 1 commit into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions components/formats-api/src/loci/formats/FormatReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -524,23 +524,23 @@ protected byte[] readPlane(RandomAccessInputStream s, int x, int y,
}
else if (x == 0 && w == getSizeX() && scanlinePad == 0) {
if (isInterleaved()) {
s.skipBytes(y * w * bpp * c);
s.skipBytes((long) y * w * bpp * c);
s.read(buf, 0, h * w * bpp * c);
}
else {
int rowLen = w * bpp;
for (int channel=0; channel<c; channel++) {
s.skipBytes(y * rowLen);
s.skipBytes((long) y * rowLen);
s.read(buf, channel * h * rowLen, h * rowLen);
if (channel < c - 1) {
// no need to skip bytes after reading final channel
s.skipBytes((getSizeY() - y - h) * rowLen);
s.skipBytes((long) (getSizeY() - y - h) * rowLen);
}
}
}
}
else {
int scanlineWidth = getSizeX() + scanlinePad;
long scanlineWidth = getSizeX() + scanlinePad;
if (isInterleaved()) {
s.skipBytes(y * scanlineWidth * bpp * c);
for (int row=0; row<h; row++) {
Expand Down
3 changes: 2 additions & 1 deletion components/formats-bsd/src/loci/formats/in/AVIReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
pad = bytes;
}

in.skipBytes((getSizeX() + pad) * (bmpBitsPerPixel / 8) * (getSizeY() - h - y));
long skipRows = getSizeY() - h - y;
in.skipBytes((getSizeX() + pad) * (bmpBitsPerPixel / 8) * skipRows);

if (getSizeX() == w && pad == 0) {
for (int row=0; row<h; row++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ private String getHeaderInfo(int tag, String value) throws IOException {
if (skip) {
long skipCount = (long) elementLength;
if (in.getFilePointer() + skipCount <= in.length()) {
in.skipBytes((int) skipCount);
in.skipBytes(skipCount);
}
location += elementLength;
value = "";
Expand Down
2 changes: 1 addition & 1 deletion components/formats-bsd/src/loci/formats/in/EPSReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
}

byte[] b = new byte[w * h];
int bpp = (int) (byteCounts[0] / (getSizeX() * getSizeY()));
long bpp = byteCounts[0] / (getSizeX() * getSizeY());
in.skipBytes(bpp * y * getSizeX());
for (int row=0; row<h; row++) {
in.skipBytes(x * bpp);
Expand Down
4 changes: 2 additions & 2 deletions components/formats-bsd/src/loci/formats/tiff/TiffParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,8 @@ public long[] getIFDOffsets() throws IOException {
while (offset > 0 && offset < in.length()) {
in.seek(offset);
offsets.add(offset);
int nEntries = bigTiff ? (int) in.readLong() : in.readUnsignedShort();
int entryBytes = nEntries * bytesPerEntry;
long nEntries = bigTiff ? in.readLong() : in.readUnsignedShort();
long entryBytes = nEntries * bytesPerEntry;
if (in.getFilePointer() + entryBytes + (bigTiff ? 8 : 4) > in.length()) {
// this can easily happen when writing multiple planes to a file
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
// so instead of LMLMLM... storage, we have LLLLL...MMMMM...
for (int i=0; i<numBytes; i++) {
in.seek(textureOffset + (no * planeSize * (i + 1)));
in.skipBytes(y * (getSizeX() + pad));
in.skipBytes((long) y * (getSizeX() + pad));
if (getSizeX() == w) {
in.read(buf, i * w * h, w * h);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ else if (in.length() - planeSize > 61000) {
int bpp = FormatTools.getBytesPerPixel(getPixelType());
int pixel = bpp * getSizeC();

in.skipBytes(pixel * getSizeX() * (getSizeY() - h - y));
in.skipBytes((long) pixel * getSizeX() * (getSizeY() - h - y));

for (int row=h-1; row>=0; row--) {
in.skipBytes(x * pixel);
Expand Down
4 changes: 2 additions & 2 deletions components/formats-gpl/src/loci/formats/in/BioRadReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ protected void initFile(String id) throws FormatException, IOException {
// skip image data
int imageLen = getSizeX() * getSizeY();
int bpp = FormatTools.getBytesPerPixel(getPixelType());
in.skipBytes(bpp * getImageCount() * imageLen + 6);
in.skipBytes((long) bpp * getImageCount() * imageLen + 6);

m.sizeZ = getImageCount();
m.sizeC = 1;
Expand Down Expand Up @@ -538,7 +538,7 @@ private void readNotes(RandomAccessInputStream s, boolean add)
throws IOException
{
s.seek(70);
int imageLen = getSizeX() * getSizeY();
long imageLen = getSizeX() * getSizeY();
if (picFiles == null) imageLen *= getImageCount();
else {
imageLen *= (getImageCount() / picFiles.length);
Expand Down
4 changes: 2 additions & 2 deletions components/formats-gpl/src/loci/formats/in/HISReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ protected void initFile(String id) throws FormatException, IOException {
if (getBitsPerPixel() == 12) {
core.get(i - 1).bitsPerPixel = 16;

int prevSkip = (getSizeX() * getSizeY() * getSizeC() * 12) / 8;
int totalBytes = FormatTools.getPlaneSize(this);
long prevSkip = ((long) getSizeX() * getSizeY() * getSizeC() * 12) / 8;
long totalBytes = FormatTools.getPlaneSize(this);
in.skipBytes(totalBytes - prevSkip);
adjustedBitDepth = true;
}
Expand Down
2 changes: 1 addition & 1 deletion components/formats-gpl/src/loci/formats/in/IMODReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ protected void initFile(String id) throws FormatException, IOException {
int surface = in.readShort();

// TODO
in.skipBytes(12 * vsize + 4 * lsize);
in.skipBytes((long) 12 * vsize + (long) 4 * lsize);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ else if (tag.equals("roi ") &&
store.setROIID(roiID, 0);
store.setImageROIRef(roiID, 0, 0);

in.skipBytes(8 * numRoiPts);
in.skipBytes((long) 8 * numRoiPts);
}
else if (tag.equals("mask")) {
// read in Segmentation Mask
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ protected void initFile(String id) throws FormatException, IOException {
}
m.pixelType = FormatTools.UINT16;
int bpp = FormatTools.getBytesPerPixel(m.pixelType);
in.skipBytes(m.sizeX * m.sizeY * planesPerBlock.get(0) * bpp + 2);
in.skipBytes((long) m.sizeX * m.sizeY * planesPerBlock.get(0) * bpp + 2);

int tileX = 1;
int tileY = 1;
Expand Down Expand Up @@ -655,7 +655,7 @@ private void findOffset(boolean readStackHeader) throws IOException {
planesPerBlock.get(planesPerBlock.size() - 1) * 2;
if (planeSize > 0 && in.getFilePointer() + planeSize < in.length()) {
pixelsOffsets.add(in.getFilePointer());
in.skipBytes((int) planeSize + 2);
in.skipBytes(planeSize + 2);
}
else {
planesPerBlock.remove(planesPerBlock.size() - 1);
Expand Down
8 changes: 4 additions & 4 deletions components/formats-gpl/src/loci/formats/in/IvisionReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
{
FormatTools.checkPlaneParameters(this, no, buf.length, x, y, w, h);

int planeSize = getSizeX() * getSizeY() * getSizeC();
long planeSize = (long) getSizeX() * getSizeY() * getSizeC();
if (color16) planeSize = 2 * (planeSize / 3);
else if (squareRoot) planeSize *= 2;
else if (hasPaddingByte) {
Expand All @@ -129,15 +129,15 @@ else if (squareRoot) {
}
else if (hasPaddingByte) {
int next = 0;
in.skipBytes(y * getSizeX() * getSizeC());
in.skipBytes((long) y * getSizeX() * getSizeC());
for (int row=0; row<h; row++) {
in.skipBytes(x * getSizeC());
in.skipBytes((long) x * getSizeC());
for (int col=0; col<w; col++) {
in.skipBytes(1);
in.read(buf, next, getSizeC());
next += getSizeC();
}
in.skipBytes(getSizeC() * (getSizeX() - w - x));
in.skipBytes((long) getSizeC() * (getSizeX() - w - x));
}
}
else readPlane(in, x, y, w, h, buf);
Expand Down
2 changes: 1 addition & 1 deletion components/formats-gpl/src/loci/formats/in/LIFReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
long planeSize = (long) getSizeX() * getSizeY() * bpp;
long nextOffset = index + 1 < offsets.size() ?
offsets.get(index + 1).longValue() : endPointer;
int bytesToSkip = (int) (nextOffset - offset - planeSize * getImageCount());
long bytesToSkip = nextOffset - offset - planeSize * getImageCount();
bytesToSkip /= getSizeY();
if ((getSizeX() % 4) == 0) bytesToSkip = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ private void parseDimensionTag(int seriesIndex)
addSeriesMeta("Maximum voxel intensity", getString(true));
addSeriesMeta("Minimum voxel intensity", getString(true));
int len = in.readInt();
in.skipBytes(len * 2 + 4);
in.skipBytes((long) len * 2 + 4);

len = in.readInt();
for (int j=0; j<len; j++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ else if (TypeUINT12) {
int thisSeries = getSeries();
for (int i=0; i<thisSeries; i++) {
setSeries(i);
in.skipBytes(getImageCount() * FormatTools.getPlaneSize(this));
in.skipBytes((long) getImageCount() * FormatTools.getPlaneSize(this));
Copy link
Member

@sbesson sbesson Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have impacted the non-regression tests for one li-flim sample file - see https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-folder/52532/console. We might need to look at the data with or without this PR to establish whether the configuration needs to be regenerated or there is an issue with this change.

Copy link
Member Author

@melissalinkert melissalinkert Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonably certain that the configuration needs to be updated, as proposed here: https://github.com/openmicroscopy/data_repo_config/pull/445

Note that without that change, the MD5s for series 0 and series 1 were the same, which points to this PR fixing a problem.

}
setSeries(thisSeries);
readPlane(in, x, y, w, h, buf);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1680,7 +1680,7 @@ private void parseUIC4Tags(long uic4offset) throws IOException {
hasAbsoluteZValid = true;
break;
case 46:
in.skipBytes(mmPlanes * 8); // TODO
in.skipBytes((long) mmPlanes * 8); // TODO
break;
default:
in.skipBytes(4);
Expand Down
2 changes: 1 addition & 1 deletion components/formats-gpl/src/loci/formats/in/NAFReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ protected void initFile(String id) throws FormatException, IOException {
String name = in.readCString();

if (i == 0) {
in.skipBytes((int) (92 - in.getFilePointer() + pointer));
in.skipBytes(92 - in.getFilePointer() + pointer);
while (true) {
int check = in.readInt();
if (check > in.getFilePointer()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ protected void initFile(String id) throws FormatException, IOException {
in.skipBytes(16);
long pointer = in.getFilePointer();
planes[imagesFound].planeName = readCString().trim();
in.skipBytes((int) (256 - in.getFilePointer() + pointer));
in.skipBytes(256 - in.getFilePointer() + pointer);

planes[imagesFound].planeOffset = in.getFilePointer();

Expand Down
2 changes: 1 addition & 1 deletion components/formats-gpl/src/loci/formats/in/PSDReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ protected void initFile(String id) throws FormatException, IOException {
int[] lens = new int[h[i]];
for (int cc=0; cc<c[i]; cc++) {
boolean compressed = in.readShort() == 1;
if (!compressed) in.skipBytes(w[i] * h[i]);
if (!compressed) in.skipBytes((long) w[i] * h[i]);
else {
for (int y=0; y<h[i]; y++) {
lens[y] = in.readShort();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ else if (sourceData instanceof TiffIFDEntry) {
addGlobalMetaList("Layer name", layerNames[layer]);
core.add(layerCore);
}
tag.skipBytes((int) (fp + len - tag.getFilePointer()));
tag.skipBytes(fp + len - tag.getFilePointer());
}

nLayers = core.size() - 1;
Expand Down Expand Up @@ -282,7 +282,7 @@ else if (compression[layer] == PACKBITS) {
}
}
}
else tag.skipBytes(length + skip);
else tag.skipBytes((long) length + skip);
}

MetadataStore store = makeFilterMetadata();
Expand Down
11 changes: 6 additions & 5 deletions components/formats-gpl/src/loci/formats/in/SDTReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
if (info.mcstaPoints == getSizeT()) {
times = getSizeT();
}
int planeSize = paddedWidth * sizeY * times * bpp;
long planeSize = (long) paddedWidth * sizeY * times * bpp;

// remove width padding if we can be reasonably certain
// that the unpadded width is correct
Expand Down Expand Up @@ -316,11 +316,12 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
codec.skip(channel * planeSize + y * paddedWidth * bpp * times);
}
else {
in.skipBytes(channel * planeSize + y * paddedWidth * bpp * times);
in.skipBytes(
channel * planeSize + (long) y * paddedWidth * bpp * times);
}

for (int row = 0; row < h; row++) {
readPixels(rowBuf, in, codec, x * bpp * times);
readPixels(rowBuf, in, codec, (long) x * bpp * times);
if (intensity) {
System.arraycopy(rowBuf, 0, b, row * bpp * times * w, b.length);
}
Expand All @@ -334,7 +335,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
}
}
if (codec == null) {
in.skipBytes(bpp * times * (paddedWidth - x - w));
in.skipBytes((long) bpp * times * (paddedWidth - x - w));
}
else {
codec.skip(bpp * times * (paddedWidth - x - w));
Expand Down Expand Up @@ -469,7 +470,7 @@ protected void initFile(String id) throws FormatException, IOException {
MetadataTools.populatePixels(store, this);
}

private void readPixels(byte[] rowBuf, RandomAccessInputStream in, ZipInputStream codec, int skip)
private void readPixels(byte[] rowBuf, RandomAccessInputStream in, ZipInputStream codec, long skip)
throws IOException
{
if (codec == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ else if (n == 'd') {
in.seek(in.getFilePointer() - 1);
long nSkipped = in.getFilePointer() - fp;
if (nSkipped < 8) {
in.skipBytes((int) (8 - nSkipped));
in.skipBytes(8 - nSkipped);
}
String objective = readCString().trim();
in.seek(fp + 144);
Expand Down
4 changes: 2 additions & 2 deletions components/formats-gpl/src/loci/formats/in/TargaReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
while (bpp % 8 != 0) bpp++;
bpp /= 8;

int rowSkip = orientation < 2 ? (getSizeY() - h - y) : y;
int colSkip = (orientation % 2) == 1 ? (getSizeX() - w - x) : x;
long rowSkip = orientation < 2 ? (getSizeY() - h - y) : y;
long colSkip = (orientation % 2) == 1 ? (getSizeX() - w - x) : x;

try {
s.skipBytes(rowSkip * getSizeX() * bpp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
s.order(isLittleEndian());
s.seek(pixelOffsets[fileIndex]);

int paddingBytes =
(int) (s.length() - s.getFilePointer() - div * plane) / (div - 1);
long paddingBytes =
(s.length() - s.getFilePointer() - div * plane) / (div - 1);
if (planeIndex > 0) {
s.skipBytes((plane + paddingBytes) * planeIndex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public byte[] openBytes(int no, byte[] buf, int x, int y, int w, int h)
FormatTools.checkPlaneParameters(this, no, buf.length, x, y, w, h);

in.seek(offsets.get(getSeriesCount() - getSeries() - 1));
in.skipBytes(no * FormatTools.getPlaneSize(this));
in.skipBytes((long) no * FormatTools.getPlaneSize(this));
readPlane(in, x, y, w, h, buf);

return buf;
Expand Down Expand Up @@ -144,7 +144,7 @@ public void initFile(String id) throws FormatException, IOException {
seekToNextMarker();
in.skipBytes(50);
offsets.add(in.getFilePointer());
in.skipBytes(thumb.sizeX * thumb.sizeY * thumb.sizeC);
in.skipBytes((long) thumb.sizeX * thumb.sizeY * thumb.sizeC);

seekToNextMarker();
in.skipBytes(50);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ private void parseROIs(RandomAccessInputStream s, int imageNum, String name, Met
int length = s.readInt();

Shape nshape = new Shape();
s.skipBytes(length + 6);
s.skipBytes((long) length + 6);

long shapeAttrOffset = s.getFilePointer();
int shapeAttrLength = s.readInt();
Expand Down