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

bug report #12

Open
dzzie opened this issue Jul 10, 2023 · 0 comments
Open

bug report #12

dzzie opened this issue Jul 10, 2023 · 0 comments

Comments

@dzzie
Copy link
Contributor

dzzie commented Jul 10, 2023

Hi, small bug to be aware of in libdasm.

from the read me:

int get_instruction_string(INSTRUCTION *instr,  enum Format format,  DWORD offset, char *string,   int length);

string is the pointer to instruction buffer, length is the size of the buffer. 

**Note that the text is truncated if it doesn't fit in buffer.**

get_instruction_string will initialize the string **and terminate it correctly for convenience**. 

It returns zero if the operation is not successful.

On windows anyway (VS2008 and VS2019 for me possibly others)

passing in to small of a buffer will lead to an access violation.
get_operand_string will be passed a negative length, which memset takes as a huge number and walks off the end of the allocation.

int get_operand_string(INSTRUCTION *inst, OPERAND *op,
	enum Format format, DWORD offset, char *string, int length) {
	
	enum Mode mode;
	int regtype = 0;
	DWORD tmp = 0;

	//if(length < 0) return 0;  //quick bugfix
	memset(string, 0, length);

So rather than the text being truncated we just get an access violation. The quick fix above will prevent the crash, but unfortunately
snprintf on windows is stupid and does not properly truncate the string with a null terminator.

https://stackoverflow.com/questions/7706936/is-snprintf-always-null-terminating

it might be worth including the length check just to help prevent crashs, the windows specific snprintf behavior isnt really worth fixing,

end of the day just be aware and use a large enough buffer to hold the full disassembly.

Below is a sample submitted to me of an extra long disasm that hit me. You can play with buffer size to test its behavior at different lengths like 10 that fall on various boundaries while the disasm is being build up incrementally and how it can fail at different spots.

void main(void){

 	INSTRUCTION inst;
	char buf[12]; //full disasm requires 34 chars + null

	//test byte [ecx+eax-0x6f6f6f70],0x0   (34 chars)  - Rafael Pedrero
	unsigned char* longInst = (unsigned char*)"\xF6\x84\x01\x90\x90\x90\x90\x00\xCC";

	int instrsize = get_instruction(&inst, longInst, MODE_32);  
	int ret = get_instruction_string(&inst, FORMAT_INTEL, 0, buf, sizeof(buf)); 

	printf("i = %d, ret=%d\ndisasm = %s", instrsize, ret, buf);
	getch();
	
}

not sure what the max disasm length would be for libdasm, the above can even be expanded a bit more with a dword

F78408 44332211 88776655     TEST DWORD [EAX+ECX+11223344],55667788  (38 chars)
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

1 participant