Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ abstract class AbstractFile : Closeable {
}

override fun toString(): String {
if (fileContents == null) {
if (!::fileContents.isInitialized) {
return "The file has not been configured. You should setup manually in the first page before you can see the details."
}
val builder = StringBuilder(
if (this is RawFile) "The file has not been configured. You should setup manually in the first page before you can see the details." +
System.lineSeparator() else ""
)
builder.append(/*R.getString(R.string.FileSize)*/"File Size:")
.append(Integer.toHexString(fileContents.size))
.append(java.lang.Long.toHexString(getBinaryLength()))
.append(ls)
Comment on lines 23 to 33

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't treat deferred bytes as "not configured".

Line 24 now returns the placeholder string whenever fileContents is still lazy. After this PR, RawFile, ElfFile, and PEFile intentionally leave fileContents uninitialized until bytes are requested, so toString() skips the size/address metadata even though getBinaryLength() and the parsed fields are already available.

🩹 Proposed fix
     override fun toString(): String {
-        if (!::fileContents.isInitialized) {
-            return "The file has not been configured. You should setup manually in the first page before you can see the details."
-        }
         val builder = StringBuilder(
             if (this is RawFile) "The file has not been configured. You should setup manually in the first page before you can see the details." +
                     System.lineSeparator() else ""
         )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/kyhsgeekcode/disassembler/files/AbstractFile.kt` around
lines 23 - 33, The toString() method incorrectly treats an uninitialized
deferred property fileContents as "not configured" and returns a placeholder;
remove that early ::fileContents.isInitialized check and let toString() always
emit available metadata (use getBinaryLength(), parsed fields) even when
fileContents is lazy-loaded; if RawFile still needs a special prefix, gate only
that prefix on the concrete type (this is RawFile) rather than on fileContents
initialization, ensuring ElfFile/PEFile and others show size/address info
regardless of fileContents state.

builder.append(appCtx.getString(R.string.FoffsCS))
.append(java.lang.Long.toHexString(codeSectionBase))
Expand Down Expand Up @@ -74,9 +74,48 @@ abstract class AbstractFile : Closeable {
@JvmField
var path = ""

open fun getBinaryContents(): ByteArray = fileContents

open fun getBinaryLength(): Long = getBinaryContents().size.toLong()

companion object {
private const val TAG = "AbstractFile"

@JvmStatic
internal fun readFileContentsForParsing(
file: File,
readerFactory: (File) -> BinaryRangeReader = ::FileChannelBinaryRangeReader
): ByteArray {
return readerFactory(file).use { reader ->
reader.readFully()
}
}

@JvmStatic
internal fun detectBinaryContainerFormat(
file: File,
readerFactory: (File) -> BinaryRangeReader = ::FileChannelBinaryRangeReader
): BinaryContainerFormat {
val header = readerFactory(file).use { reader ->
reader.read(offset = 0, length = 4)
}
if (header.size >= 4 &&
header[0] == 0x7F.toByte() &&
header[1] == 'E'.code.toByte() &&
header[2] == 'L'.code.toByte() &&
header[3] == 'F'.code.toByte()
) {
return BinaryContainerFormat.ELF
}
if (header.size >= 2 &&
header[0] == 'M'.code.toByte() &&
header[1] == 'Z'.code.toByte()
) {
return BinaryContainerFormat.PE
}
return BinaryContainerFormat.RAW
}

@JvmStatic
@Throws(IOException::class)
fun createInstance(file: File): AbstractFile {
Expand All @@ -89,8 +128,8 @@ abstract class AbstractFile : Closeable {
// 그리고 AfterReadFully 함수는 없어질지도 모른다!
// 그러면 중복코드도 사라짐
// 행복회로
val content = file.readBytes()
if (file.path.endsWith("assets/bin/Data/Managed/Assembly-CSharp.dll")) { // Unity C# dll file
val content = readFileContentsForParsing(file)
Logger.v(TAG, "Found C# unity dll")
try {
val facileReflector = Facile.load(file.path)
Expand All @@ -109,30 +148,45 @@ abstract class AbstractFile : Closeable {
} catch (e: SizeMismatchException) {
e.printStackTrace()
}
} else {
return try {
ElfFile(file, content)
} catch (e: Exception) { // not an elf file. try PE parser
Timber.d(e, "Fail elfutil")
}
return when (detectBinaryContainerFormat(file)) {
BinaryContainerFormat.ELF -> {
try {
ElfFile(file, filec = null, deferredContentLoader = BinaryContentLoader {
readFileContentsForParsing(file)
})
} catch (e: Exception) {
Timber.d(e, "Fail elfutil")
RawFile(file, filecontent = null) {
readFileContentsForParsing(file)
}
}
}

BinaryContainerFormat.PE -> {
val content = readFileContentsForParsing(file)
try {
PEFile(file, content)
PEFile(file, content, BinaryContentLoader {
readFileContentsForParsing(file)
})
} catch (f: NotThisFormatException) {
Timber.e(f, "Not this format exception")
RawFile(file, content)
// AllowRawSetup();
// failed to parse the file. please setup manually.
} catch (f: RuntimeException) { // AlertError("Failed to parse the file. Please setup manually. Sending an error report, the file being analyzed can be attached.", f);
} catch (f: RuntimeException) {
Timber.e(f, "Not this format exception")
RawFile(file, content)
// AllowRawSetup();
} catch (g: Exception) { // AlertError("Unexpected exception: failed to parse the file. please setup manually.", g);
} catch (g: Exception) {
Timber.e(g, "What the exception")
RawFile(file, content)
// AllowRawSetup();
}
}

BinaryContainerFormat.RAW -> {
RawFile(file, filecontent = null) {
readFileContentsForParsing(file)
}
}
}
return RawFile(file, content)
// return null
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.kyhsgeekcode.disassembler.files

enum class BinaryContainerFormat {
ELF,
PE,
RAW,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.kyhsgeekcode.disassembler.files

fun interface BinaryContentLoader {
fun load(): ByteArray
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.kyhsgeekcode.disassembler.files

import java.io.Closeable

interface BinaryRangeReader : Closeable {
val size: Long

fun read(offset: Long, length: Int): ByteArray

fun readFully(): ByteArray
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.kyhsgeekcode.disassembler.files

import java.io.File

class DeferredFileBackedBinaryContent(
private val file: File,
initialContent: ByteArray? = null,
private val loader: BinaryContentLoader? = null,
) {
private var loadedContent: ByteArray? = initialContent

fun contents(): ByteArray {
val existing = loadedContent
if (existing != null) {
return existing
}
val loaded = loader?.load() ?: file.readBytes()
loadedContent = loaded
return loaded
}

fun length(): Long {
return loadedContent?.size?.toLong() ?: file.length()
}
}
30 changes: 27 additions & 3 deletions app/src/main/java/com/kyhsgeekcode/disassembler/files/ElfFile.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,18 @@ import java.io.IOException
import java.nio.ByteBuffer
import java.util.*

class ElfFile(file: File, filec: ByteArray) : AbstractFile() {
class ElfFile(
private val file: File,
filec: ByteArray? = null,
deferredContentLoader: BinaryContentLoader? = null,
) : AbstractFile() {
private var contentLoaded = filec != null
private val binaryContent = DeferredFileBackedBinaryContent(
file = file,
initialContent = filec,
loader = deferredContentLoader,
)

fun getPltIndexFromJumpAddress(address: Long): Int {
Log.d(TAG, "GetPltIndexFromJumpAddress $address")
pltRange?.let {
Expand Down Expand Up @@ -53,6 +64,18 @@ class ElfFile(file: File, filec: ByteArray) : AbstractFile() {
elf.close()
}

override fun getBinaryContents(): ByteArray {
if (!contentLoaded) {
fileContents = binaryContent.contents()
contentLoaded = true
}
return fileContents
}

override fun getBinaryLength(): Long {
return if (contentLoaded) fileContents.size.toLong() else binaryContent.length()
}

override fun toString(): String {
val sb = StringBuilder(super.toString())
sb.append(System.lineSeparator())
Expand Down Expand Up @@ -515,8 +538,9 @@ class ElfFile(file: File, filec: ByteArray) : AbstractFile() {
init {
elf = Elf(file)
path = file.path
fileContents = filec
if (filec != null) {
fileContents = filec
}
afterConstructor()
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.kyhsgeekcode.disassembler.files

import java.io.File
import java.nio.ByteBuffer
import java.nio.channels.FileChannel
import java.nio.file.StandardOpenOption
import kotlin.math.min

class FileChannelBinaryRangeReader(file: File) : BinaryRangeReader {
private val channel = FileChannel.open(file.toPath(), StandardOpenOption.READ)

override val size: Long
get() = channel.size()

override fun read(offset: Long, length: Int): ByteArray {
require(offset >= 0) { "offset must be non-negative" }
require(length >= 0) { "length must be non-negative" }
if (length == 0 || offset >= size) {
return ByteArray(0)
}
val readableLength = min(length.toLong(), size - offset).toInt()
val buffer = ByteBuffer.allocate(readableLength)
channel.read(buffer, offset)
return buffer.array()
}

override fun readFully(): ByteArray {
if (size == 0L) {
return ByteArray(0)
}
require(size <= Int.MAX_VALUE) {
"File is too large to fit into a ByteArray: $size bytes"
}
return read(offset = 0, length = size.toInt())
}
Comment on lines +15 to +35

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does java.nio.channels.FileChannel.read(ByteBuffer, long) guarantee that a single call fills the ByteBuffer for regular files, or can it legally return fewer bytes than requested?

💡 Result:

No, java.nio.channels.FileChannel.read(ByteBuffer, long) does not guarantee that a single call fills the ByteBuffer, even for regular files. It can legally return fewer bytes than requested.

Citations:


Handle short reads instead of assuming one FileChannel.read fills the buffer.

The channel.read(buffer, offset) call at line 23 ignores the returned byte count. Since FileChannel.read() can legally return fewer bytes than requested, even for regular files, a short read leaves the buffer partially uninitialized (zero-padded). This corrupts readFully() at line 34, which inherits the same defect.

Use a loop to read until the buffer is full or EOF:

Proposed fix
     override fun read(offset: Long, length: Int): ByteArray {
         require(offset >= 0) { "offset must be non-negative" }
         require(length >= 0) { "length must be non-negative" }
         if (length == 0 || offset >= size) {
             return ByteArray(0)
         }
         val readableLength = min(length.toLong(), size - offset).toInt()
         val buffer = ByteBuffer.allocate(readableLength)
-        channel.read(buffer, offset)
-        return buffer.array()
+        var position = offset
+        while (buffer.hasRemaining()) {
+            val bytesRead = channel.read(buffer, position)
+            if (bytesRead <= 0) {
+                break
+            }
+            position += bytesRead
+        }
+        return buffer.array().copyOf(buffer.position())
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
override fun read(offset: Long, length: Int): ByteArray {
require(offset >= 0) { "offset must be non-negative" }
require(length >= 0) { "length must be non-negative" }
if (length == 0 || offset >= size) {
return ByteArray(0)
}
val readableLength = min(length.toLong(), size - offset).toInt()
val buffer = ByteBuffer.allocate(readableLength)
channel.read(buffer, offset)
return buffer.array()
}
override fun readFully(): ByteArray {
if (size == 0L) {
return ByteArray(0)
}
require(size <= Int.MAX_VALUE) {
"File is too large to fit into a ByteArray: $size bytes"
}
return read(offset = 0, length = size.toInt())
}
override fun read(offset: Long, length: Int): ByteArray {
require(offset >= 0) { "offset must be non-negative" }
require(length >= 0) { "length must be non-negative" }
if (length == 0 || offset >= size) {
return ByteArray(0)
}
val readableLength = min(length.toLong(), size - offset).toInt()
val buffer = ByteBuffer.allocate(readableLength)
var position = offset
while (buffer.hasRemaining()) {
val bytesRead = channel.read(buffer, position)
if (bytesRead <= 0) {
break
}
position += bytesRead
}
return buffer.array().copyOf(buffer.position())
}
override fun readFully(): ByteArray {
if (size == 0L) {
return ByteArray(0)
}
require(size <= Int.MAX_VALUE) {
"File is too large to fit into a ByteArray: $size bytes"
}
return read(offset = 0, length = size.toInt())
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@app/src/main/java/com/kyhsgeekcode/disassembler/files/FileChannelBinaryRangeReader.kt`
around lines 15 - 35, The read(offset: Long, length: Int) implementation assumes
channel.read(buffer, offset) fills the buffer; change it to handle short reads
by looping on channel.read until the buffer has no remaining space or EOF (-1)
is returned, accumulating the total bytes read and stopping when reached
readableLength; then return only the bytes actually read (use buffer.position()
or similar) so partial reads don't leave zero-padded data. Update readFully() to
rely on the corrected read(...) behavior for large reads and ensure you respect
the readableLength computed in read(offset,length) and break the loop on -1 to
avoid infinite loops.


override fun close() {
channel.close()
}
}
49 changes: 41 additions & 8 deletions app/src/main/java/com/kyhsgeekcode/disassembler/files/PEFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,22 @@
public class PEFile extends AbstractFile {
PE pe;
ArrayList<TLS> tlss = new ArrayList<>();
private final DeferredFileBackedBinaryContent binaryContent;
private boolean contentLoaded;

public PEFile(File file, byte[] filec) throws IOException, NotThisFormatException {
this(file, filec, null);
}

public PEFile(File file, byte[] filec, BinaryContentLoader deferredContentLoader) throws IOException, NotThisFormatException {
path = file.getPath();
byte[] parsingBytes = filec != null ? filec : loadBinaryContents(file, deferredContentLoader);
binaryContent = new DeferredFileBackedBinaryContent(
file,
deferredContentLoader == null ? parsingBytes : null,
deferredContentLoader
);
contentLoaded = deferredContentLoader == null;
Comment on lines 36 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Initialize fileContents before marking the PE payload as loaded.

When deferredContentLoader is null, Line 48 sets contentLoaded = true, but no value is assigned to fileContents. Any caller that still uses the two-arg constructor on Line 36 will then hit an uninitialized/null buffer in getBinaryContents() and getBinaryLength().

🐛 Proposed fix
     byte[] parsingBytes = filec != null ? filec : loadBinaryContents(file, deferredContentLoader);
     binaryContent = new DeferredFileBackedBinaryContent(
             file,
             deferredContentLoader == null ? parsingBytes : null,
             deferredContentLoader
     );
-    contentLoaded = deferredContentLoader == null;
+    if (deferredContentLoader == null) {
+        fileContents = parsingBytes;
+        contentLoaded = true;
+    } else {
+        contentLoaded = false;
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/kyhsgeekcode/disassembler/files/PEFile.java` around
lines 36 - 48, The constructor PEFile(File file, byte[] filec,
BinaryContentLoader deferredContentLoader) sets contentLoaded = true when
deferredContentLoader is null but never assigns fileContents, leaving
getBinaryContents()/getBinaryLength() to read a null buffer; fix by initializing
the instance field fileContents with parsingBytes (or a copy) before setting
contentLoaded when deferredContentLoader == null (inside the same constructor),
ensuring DeferredFileBackedBinaryContent is created as before and then
fileContents is populated so getBinaryContents()/getBinaryLength() are safe.

try {
pe = PEParser.parse(file);
} catch (NegativeArraySizeException e) {
Expand All @@ -54,7 +67,6 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio
setCodeSectionLimit(getCodeSectionBase() + oph.getSizeOfCode());
setCodeVirtAddr(oph.getImageBase() + getCodeSectionBase());
setEntryPoint(oph.getAddressOfEntryPoint());
fileContents = filec;

getExportSymbols().clear();
getImportSymbols().clear();
Expand All @@ -68,11 +80,11 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio
if (ide == null)
continue;//null dll

String dllname = Elf.getZString(filec, rvc.convertVirtualAddressToRawDataPointer(ide.getNameRVA()));//idir.getName(i);//get dll name? !! Not implemented method!!!!
String dllname = Elf.getZString(parsingBytes, rvc.convertVirtualAddressToRawDataPointer(ide.getNameRVA()));//idir.getName(i);//get dll name? !! Not implemented method!!!!
Timber.v(dllname);
long originalFirstThunkRaw = rvc.convertVirtualAddressToRawDataPointer(ide.getImportLookupTableRVA());//OriginalFirstThunk
long firstThunkRaw = rvc.convertVirtualAddressToRawDataPointer(ide.getImportAddressTableRVA());
ByteBuffer buf = ByteBuffer.wrap(filec, (int) originalFirstThunkRaw, (int) (filec.length - originalFirstThunkRaw)).order(ByteOrder.LITTLE_ENDIAN);
ByteBuffer buf = ByteBuffer.wrap(parsingBytes, (int) originalFirstThunkRaw, (int) (parsingBytes.length - originalFirstThunkRaw)).order(ByteOrder.LITTLE_ENDIAN);
int off = 0;
//Read by dword!
for (; ; ) {
Expand All @@ -89,7 +101,7 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio
//CHAR name[1];
/*ByteBuffer INT=ByteBuffer.wrap(filec,(int)data,(int)(filec.length-data));
INT.getShort();*/
String funcname = Elf.getZString(filec, rvc.convertVirtualAddressToRawDataPointer((int) data) + 2);
String funcname = Elf.getZString(parsingBytes, rvc.convertVirtualAddressToRawDataPointer((int) data) + 2);
//Log.v(TAG,dllname+"."+funcname);
importSymbol.owner = dllname;
importSymbol.name = funcname;
Expand All @@ -107,16 +119,16 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio
long funcAddrRaw = rvc.convertVirtualAddressToRawDataPointer((int) edir.getExportAddressTableRVA());
long funcNameRaw = rvc.convertVirtualAddressToRawDataPointer((int) edir.getNamePointerRVA());
long funcOrdinalRaw = rvc.convertVirtualAddressToRawDataPointer((int) edir.getOrdinalTableRVA());
ByteBuffer funcnamePointers = ByteBuffer.wrap(filec, (int) funcNameRaw, (int) (filec.length - funcNameRaw)).order(ByteOrder.LITTLE_ENDIAN);//len eq num of name
ByteBuffer funcOrdinalPointers = ByteBuffer.wrap(filec, (int) funcOrdinalRaw, (int) (filec.length - funcOrdinalRaw)).order(ByteOrder.LITTLE_ENDIAN);//len eq num of name
ByteBuffer funcnamePointers = ByteBuffer.wrap(parsingBytes, (int) funcNameRaw, (int) (parsingBytes.length - funcNameRaw)).order(ByteOrder.LITTLE_ENDIAN);//len eq num of name
ByteBuffer funcOrdinalPointers = ByteBuffer.wrap(parsingBytes, (int) funcOrdinalRaw, (int) (parsingBytes.length - funcOrdinalRaw)).order(ByteOrder.LITTLE_ENDIAN);//len eq num of name
int ordinalbase = (int) edir.getOrdinalBase();
Timber.v("OrdinalBase=" + ordinalbase);
//RVAConverter rvc=pe.getSectionTable().getRVAConverter();
for (int i = 0; i < numofExports; i++)//iterate over functions
{
Symbol sym = new Symbol();
try {
sym.name = Elf.getZString(filec, rvc.convertVirtualAddressToRawDataPointer(funcnamePointers.getInt() & 0x7FFFFFFF));
sym.name = Elf.getZString(parsingBytes, rvc.convertVirtualAddressToRawDataPointer(funcnamePointers.getInt() & 0x7FFFFFFF));
} catch (StringIndexOutOfBoundsException e) {
Timber.e(e);
sym.name = "ordinal?";
Expand All @@ -126,7 +138,7 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio
int ordinal = funcOrdinalPointers.getShort() & 0x7FFF;
long addraddr = funcAddrRaw + 4 * (ordinal - ordinalbase);
Timber.v("addraddr=" + addraddr);
sym.st_value = ByteBuffer.wrap(filec, (int) addraddr, (int) (filec.length - addraddr)).order(ByteOrder.LITTLE_ENDIAN).getInt() & 0x7FFFFFFF;
sym.st_value = ByteBuffer.wrap(parsingBytes, (int) addraddr, (int) (parsingBytes.length - addraddr)).order(ByteOrder.LITTLE_ENDIAN).getInt() & 0x7FFFFFFF;
Timber.v(sym.toString());
sym.type = Symbol.Type.STT_FUNC;
sym.bind = Symbol.Bind.STB_GLOBAL;
Expand Down Expand Up @@ -163,6 +175,27 @@ public PEFile(File file, byte[] filec) throws IOException, NotThisFormatExceptio

}

private static byte[] loadBinaryContents(File file, BinaryContentLoader deferredContentLoader) throws IOException {
if (deferredContentLoader != null) {
return deferredContentLoader.load();
}
return java.nio.file.Files.readAllBytes(file.toPath());
}

@Override
public byte[] getBinaryContents() {
if (!contentLoaded) {
fileContents = binaryContent.contents();
contentLoaded = true;
}
return fileContents;
}

@Override
public long getBinaryLength() {
return contentLoaded ? fileContents.length : binaryContent.length();
}

//https://docs.microsoft.com/ko-kr/windows/desktop/api/winnt/ns-winnt-_image_file_header
private MachineType getMachineTypeFromPE(int machine) {
int h = org.boris.pecoff4j.constant.MachineType.IMAGE_FILE_MACHINE_I386;
Expand Down
Loading
Loading