From db839765545caf825cb7ca73f4664312e7287256 Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Fri, 17 Jul 2026 08:38:44 -0400 Subject: [PATCH] GP-7071: Fixing potential infinite loop in the PefLoader, and some other PEF/OMF parsing fixes --- .../app/util/bin/format/omf/Omf2or4.java | 37 +++---------------- .../app/util/bin/format/omf/OmfUtils.java | 2 +- .../bin/format/omf/omf/OmfIteratedData.java | 18 +++++---- .../util/bin/format/pef/ContainerHeader.java | 12 ++++-- .../util/bin/format/pef/SectionHeader.java | 14 ++++--- .../ghidra/app/util/opinion/PefLoader.java | 8 ++++ 6 files changed, 43 insertions(+), 48 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/Omf2or4.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/Omf2or4.java index f21d076560..a12709c95a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/Omf2or4.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/Omf2or4.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -23,36 +23,11 @@ import ghidra.util.exception.DuplicateNameException; /** * An OMF value that is either 2 or 4 bytes + * + * @param length 2 or 4 + * @param value The 2 or 4 byte value */ -public class Omf2or4 implements StructConverter { - - private int length; - private long value; - - /** - * Creates a new {@link Omf2or4} - * - * @param length 2 or 4 - * @param value The 2 or 4 byte value - */ - public Omf2or4(int length, long value) { - this.length = length; - this.value = value; - } - - /** - * {@return the length of the value (2 or 4)} - */ - public int length() { - return length; - } - - /** - * {@return the value} - */ - public long value() { - return value; - } +public record Omf2or4(int length, long value) implements StructConverter { @Override public DataType toDataType() throws DuplicateNameException, IOException { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfUtils.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfUtils.java index a93cff8ed9..6b227b9c68 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfUtils.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/OmfUtils.java @@ -35,7 +35,7 @@ public class OmfUtils { public static final String CATEGORY_PATH = "/OMF"; public static Omf2or4 readInt2Or4(BinaryReader reader, boolean isBig) throws IOException { - return isBig ? new Omf2or4(4, reader.readNextInt()) + return isBig ? new Omf2or4(4, reader.readNextUnsignedInt()) : new Omf2or4(2, reader.readNextUnsignedShort()); } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfIteratedData.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfIteratedData.java index f6a221ea9c..faf9f03c49 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfIteratedData.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/omf/omf/OmfIteratedData.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -19,7 +19,8 @@ import java.io.IOException; import java.util.ArrayList; import ghidra.app.util.bin.BinaryReader; -import ghidra.app.util.bin.format.omf.*; +import ghidra.app.util.bin.format.omf.OmfException; +import ghidra.app.util.bin.format.omf.OmfUtils; import ghidra.program.model.data.DataType; import ghidra.util.exception.DuplicateNameException; @@ -83,14 +84,17 @@ public class OmfIteratedData extends OmfData { * Contain the definition of one part of a datablock with possible recursion */ public static class DataBlock { - private Omf2or4 repeatCount; + private int repeatCount; private int blockCount; private byte[] simpleBlock = null; private DataBlock[] nestedBlock = null; public static DataBlock read(BinaryReader reader, boolean hasBigFields) throws IOException { DataBlock subblock = new DataBlock(); - subblock.repeatCount = OmfUtils.readInt2Or4(reader, hasBigFields); + subblock.repeatCount = (int) OmfUtils.readInt2Or4(reader, hasBigFields).value(); + if (subblock.repeatCount < 0) { + throw new IOException("Iterated block has negative repeat count"); + } subblock.blockCount = reader.readNextUnsignedShort(); if (subblock.blockCount == 0) { int size = reader.readNextByte() & 0xff; @@ -115,7 +119,7 @@ public class OmfIteratedData extends OmfData { * @return The position after the block */ public int fillBuffer(byte[] buffer, int pos) { - for (int i = 0; i < (int) repeatCount.value(); ++i) { + for (int i = 0; i < repeatCount; ++i) { if (simpleBlock != null) { for (byte element : simpleBlock) { buffer[pos] = element; @@ -144,7 +148,7 @@ public class OmfIteratedData extends OmfData { length += block.getLength(); } } - return length * (int) repeatCount.value(); + return length * repeatCount; } /** diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/ContainerHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/ContainerHeader.java index 511fab80d8..d13813089d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/ContainerHeader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/ContainerHeader.java @@ -15,14 +15,14 @@ */ package ghidra.app.util.bin.format.pef; -import ghidra.app.util.bin.*; -import ghidra.program.model.data.*; -import ghidra.util.exception.DuplicateNameException; - import java.io.IOException; import java.util.ArrayList; import java.util.List; +import ghidra.app.util.bin.*; +import ghidra.program.model.data.*; +import ghidra.util.exception.DuplicateNameException; + /** * See Apple's -- PEFBinaryFormat.h *
@@ -103,6 +103,10 @@ public class ContainerHeader implements StructConverter {
 			}
 			_sections.add(section);
 		}
+
+		if (_loader == null) {
+			throw new PefException("Loader section not found!");
+		}
 	}
 
 	/**
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/SectionHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/SectionHeader.java
index 858d4e287d..d1ed148306 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/SectionHeader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pef/SectionHeader.java
@@ -4,9 +4,9 @@
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -113,8 +113,9 @@ public class SectionHeader implements StructConverter {
 	 * @param monitor the task monitor
 	 * @return the unpacked data
 	 * @throws IOException if an i/o error occurs or the section is not packed.
+	 * @throws IllegalStateException an an unexpected state occurs
 	 */
-	public byte[] getUnpackedData(TaskMonitor monitor) throws IOException {
+	public byte[] getUnpackedData(TaskMonitor monitor) throws IOException, IllegalStateException {
 		if (getSectionKind() != SectionKind.PackedData) {
 			throw new IOException("Attempt to unpack a section that is not packed.");
 		}
@@ -127,7 +128,7 @@ public class SectionHeader implements StructConverter {
 				}
 				int value = input.read();
 				if (value == -1) {
-					throw new IllegalStateException();
+					throw new IllegalStateException("Unexpectedly reached end-of-file");
 				}
 				int count = value & 0x1f;//count is the lower 5 bits...
 				if (count == 0) {
@@ -218,11 +219,14 @@ public class SectionHeader implements StructConverter {
 		}
 	}
 
-	private int unpackNextValue(InputStream input) throws IOException {
+	private int unpackNextValue(InputStream input) throws IOException, IllegalStateException {
 		int unpacked = 0;
 		while (true) {
 			unpacked <<= 7;
 			int value = input.read();
+			if (value == -1) {
+				throw new IllegalStateException("Unexpectedly reached end-of-file");
+			}
 			unpacked += (value & 0x7f);
 			if ((value & 0x80) == 0x00) {
 				break;
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PefLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PefLoader.java
index 3d274bc7e2..7b3dde1bd7 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PefLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PefLoader.java
@@ -35,6 +35,14 @@ import ghidra.program.model.symbol.*;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
+/**
+ * A {@link Loader} for Preferred Executable Format (PEF) files.
+ * 

+ * PEF was developed by Apple for use in its classic Mac OS operating system. BeOS on PowerPC + * systems also uses PEF. + * + * @see PEF Structure + */ public class PefLoader extends AbstractProgramWrapperLoader { public final static String PEF_NAME = "Preferred Executable Format (PEF)";