Add evm disassembly support. #3595

gogo2464 · 2023-06-21T09:14:56Z

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the rizin book with the relevant information (if needed)

Detailed description

Test plan

At this moment, there is mostly only the disassembly plugin that is under work.
Based on dead code. I will remove the dead code.
I have not added enough testing yet. I need to add more before merging.
I will add the plugin commit diff in the rizin book about how to code plugin with meson.

Closing issues
...

gogo2464 · 2023-06-21T09:18:31Z

Please do not review dead code.

I have issues at this line because 2 lines are printed:
6061 =>

PUSH1 0x61
PUSH2

instead of only PUSH1 0x61

What could I do please?

ret2libc

When you pass 6061, evm_op[0x60].type is GB_8BIT, thus gbOpLength(evm_op[0x60].type) returns 1, meaning that the disassembled instruction has length 1. However, given that PUSH1 has an argument specified in the next byte, it should have a length 2. That's why the 61 after the 60 is interpreted as its own instruction.

librz/asm/arch/evm/evmdis.c

gogo2464 · 2023-06-23T12:45:00Z

The testing allows me to find a bug on CALLDATACOPY lol

gogo2464 · 2023-06-23T13:02:49Z

Using tsting I realized:

-PUSH3 0x010203
+PUSH3 0x123

What should I do in your opinion @ret2libc or what else ?

gogo2464 · 2023-06-23T13:26:40Z

I have made the testing from:

echo 000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9fa0a1a2a3a4a5a6a7a8a9aaabacadaeafb0b1b2b3b4b5b6b7b8b9babbbcbdbebfc0c1c2c3c4c5c6c7c8c9cacbcccdcecfd0d1d2d3d4d5d6d7d8d9dadbdcdddedfe0e1e2e3e4e5e6e7e8e9eaebecedeeeff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff | evmasm -d

librz/asm/arch/evm/evmdis.c

librz/asm/arch/evm/evm_op_table.h

XVilka · 2023-06-23T17:05:56Z

@gogo2464 have you seen this? #3368

gogo2464 · 2023-06-23T17:30:07Z

@gogo2464 have you seen this? #3368

I have never ever seen this... this is terrible!!! I am going to check which version is better and where for both.

I think my disassembler is more tested and then more accurate than the old one. See my testing file. I also need to write my program for a school project. Merging it is not mandatory but could be a good plus.

Need your opinion.

gogo2464 · 2023-06-23T17:32:10Z

The analysis part of the other plan sounds good as well.

gogo2464 · 2023-06-23T17:35:15Z

I suggest to keep my disassemble plugin but keep his analysis plugin.

gogo2464 · 2023-06-23T17:35:26Z

@XVilka

gogo2464 · 2023-06-23T18:01:38Z

My code is very very well tested. There is no missing instruction .

librz/asm/arch/evm/evmasm.c

wargio · 2023-06-25T02:41:23Z

librz/asm/arch/evm/evmdis.c

+			instruction_size = 5;
+			strncpy(out, buf+1, instruction_size-1);
+			rz_hex_bin2str(out, instruction_size-1, opcode);


What if len is less than instruction_size ? this will create an out of bound read. you need to add checks to verify you have enough buffer before running rz_hex_bin2str and if there is not, you should return an invalid instruction.

gogo2464 · 2023-07-19T12:26:09Z

sounds good to me

gogo2464 · 2023-07-19T12:36:33Z

All is very very finely tested. May be ready for a merge.

All the instructions can be assembled/disassembled except PUSH32 that assembles with:

7f0102030405060708090102030405060708090102030405060708090102030421

instead of

7f0102030405060708090102030405060708090102030405060708090102030405

You may help me or you may merge directly.

librz/asm/arch/evm/evmasm.c

gogo2464 · 2023-07-20T11:36:28Z

Could you merge before the 21 at 4pm london utc please? It could be a great plus for a school project. Any comment is welcome before this date.

wargio · 2023-07-21T02:53:54Z

librz/asm/arch/evm/evmasm.c

+	if (!strcmp("stop", buf)) {
+		opbuf[0] = 0x00;
+	} else if (!strcmp("add", buf)) {
+		opbuf[0] = 0x01;
+	} else if (!strcmp("sub", buf)) {
+		opbuf[0] = 0x03;
+	} else if (!strcmp("div", buf)) {


i strongly suggest to make a structure with these bytes, it is easier to read and you can loop easily.

wargio · 2023-07-21T02:57:45Z

librz/asm/arch/evm/evmasm.c

+		char out[100];
+		int number = atoi(buf+4);
+		switch (number) {
+			case 1:
+			len = 2;
+			opbuf[0] = 0x60;
+			break;
+		case 2:


you can optimize this out by using a structure like suggested before which contains name, length, and the first byte.

wargio · 2023-07-21T02:59:15Z

librz/asm/arch/evm/evmasm.c

+			for (int j = 8, i = 0; i < len-1; j += 2, i ++) {
+				two_chars_str[0] = buf[j];
+				two_chars_str[1] = buf[j+1];
+				two_chars_str[2] = '\0';
+				opbuf[i+1] = strtol(two_chars_str, NULL, 16);
+			}
+	    	} else if (number > 9) {
+			for (int j = 9, i = 0; i < len-1; j += 2, i ++) {
+				two_chars_str[0] = buf[j];
+				two_chars_str[1] = buf[j+1];
+				two_chars_str[2] = '\0';
+				opbuf[i+1] = strtol(two_chars_str, NULL, 16);
+			}


there are methods in rizin to convert hex to bytes. also you don't really need to use strtol for doing that.

wargio · 2023-07-21T03:00:53Z

librz/asm/arch/evm/evmasm.c

+#include <string.h>
+#include <stdlib.h>
+
+static int evmAsm(RzAsm *a, RzAsmOp *op, const char *buf) {


You always assume that buf contains the correct information, but what happens if its length is less than expected? based on your code, many out of bound reads. you should always check for boundaries.

wargio · 2023-07-21T03:02:52Z

librz/asm/arch/evm/evmdis.c

+	const unsigned char opcode[256];
+	switch (buf[0]) {
+		case 0x00:
+			rz_str_cpy(buf_asm, "STOP") break;


There is no need to use rz_str_cpy when you have to push data into RzStrBuf.
just do rz_strbuf_set for static strings and rz_strbuf_appendf for adding extra info to the string.

wargio · 2023-07-21T03:04:35Z

librz/asm/arch/evm/evmdis.c

+			instruction_size = 21;
+			rz_hex_bin2str(buf+1, instruction_size-1, opcode);


You need to always check if the buffer is big enough. you cannot just read it as is and assume it will always be.

test/db/asm/evm

wargio · 2023-07-21T03:06:50Z

librz/asm/arch/evm/evmasm.c

@@ -0,0 +1,359 @@
+// SPDX-FileCopyrightText: 2023 gogo <[email protected]>
+// SPDX-FileCopyrightText: 2012-2020 pancake <[email protected]>


remove pancake. you wrote this, he didnt.

wargio · 2023-07-21T03:07:06Z

librz/asm/arch/evm/evmdis.c

@@ -0,0 +1,374 @@
+// SPDX-FileCopyrightText: 2023 gogo <[email protected]>
+// SPDX-FileCopyrightText: 2013-2018 pancake <[email protected]>


remove pancake, you wrote this, not him.

wargio

Please check always if the buffer is long enough. this code has many security vulnerabilities.

gogo2464 · 2023-07-28T15:55:24Z

I will review this week.

gogo2464 · 2023-08-21T15:45:01Z

sure let me fix some issues.

XVilka · 2024-01-11T12:11:21Z

I am closing this because of no activity. Please open a new one if you decide to continue.

gogo2464 · 2024-01-12T09:51:21Z

@XVilka I understand. I was confused with the table. Can I see this month if I will continue and rebase it please?

wargio · 2024-01-12T11:31:26Z

yes sure

github-actions bot added API rz-test RzAsm-Disassembler labels Jun 21, 2023

gogo2464 mentioned this pull request Jun 21, 2023

remove Makefile on dev-asm rizinorg/book#98

Merged

gogo2464 force-pushed the evm branch from e460d18 to a4520dd Compare June 21, 2023 11:23

ret2libc reviewed Jun 21, 2023

View reviewed changes

librz/asm/arch/evm/evmdis.c Outdated Show resolved Hide resolved

gogo2464 force-pushed the evm branch 5 times, most recently from bbfd5d3 to c9d5c48 Compare June 23, 2023 12:44

wargio reviewed Jun 23, 2023

View reviewed changes

librz/asm/arch/evm/evmdis.c Outdated Show resolved Hide resolved

wargio reviewed Jun 23, 2023

View reviewed changes

librz/asm/arch/evm/evmdis.c Outdated Show resolved Hide resolved

wargio reviewed Jun 23, 2023

View reviewed changes

librz/asm/arch/evm/evm_op_table.h Outdated Show resolved Hide resolved

gogo2464 force-pushed the evm branch from c9d5c48 to 25bf028 Compare June 23, 2023 16:56

gogo2464 force-pushed the evm branch from 25bf028 to 53fd58c Compare June 23, 2023 17:56

gogo2464 force-pushed the evm branch 2 times, most recently from 3cc0c1f to 6ff0ee1 Compare June 23, 2023 22:35

wargio reviewed Jun 25, 2023

View reviewed changes

librz/asm/arch/evm/evmasm.c Outdated Show resolved Hide resolved

wargio reviewed Jun 25, 2023

View reviewed changes

gogo2464 marked this pull request as ready for review July 19, 2023 12:34

gogo2464 requested review from XVilka, kazarmy and thestr4ng3r as code owners July 19, 2023 12:34

wargio reviewed Jul 20, 2023

View reviewed changes

librz/asm/arch/evm/evmasm.c Outdated Show resolved Hide resolved

gogo2464 force-pushed the evm branch 2 times, most recently from aa3a495 to d710868 Compare July 20, 2023 10:43

wargio reviewed Jul 21, 2023

View reviewed changes

test/db/asm/evm Outdated Show resolved Hide resolved

wargio reviewed Jul 21, 2023

View reviewed changes

wargio requested changes Jul 21, 2023

View reviewed changes

Add evm disassembly support.

111cf98

gogo2464 force-pushed the evm branch from d710868 to 56a3250 Compare September 2, 2023 15:39

Merge testing.

01cbd60

gogo2464 force-pushed the evm branch from 56a3250 to 01cbd60 Compare September 2, 2023 15:41

XVilka closed this Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add evm disassembly support. #3595

Add evm disassembly support. #3595

gogo2464 commented Jun 21, 2023

gogo2464 commented Jun 21, 2023

ret2libc left a comment

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

XVilka commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

wargio Jun 25, 2023

gogo2464 commented Jul 19, 2023

gogo2464 commented Jul 19, 2023

gogo2464 commented Jul 20, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio Jul 21, 2023

wargio left a comment

gogo2464 commented Jul 28, 2023

gogo2464 commented Aug 21, 2023

XVilka commented Jan 11, 2024

gogo2464 commented Jan 12, 2024

wargio commented Jan 12, 2024

		instruction_size = 21;
		rz_hex_bin2str(buf+1, instruction_size-1, opcode);

		@@ -0,0 +1,359 @@
		// SPDX-FileCopyrightText: 2023 gogo <[email protected]>
		// SPDX-FileCopyrightText: 2012-2020 pancake <[email protected]>

		@@ -0,0 +1,374 @@
		// SPDX-FileCopyrightText: 2023 gogo <[email protected]>
		// SPDX-FileCopyrightText: 2013-2018 pancake <[email protected]>

Add evm disassembly support. #3595

Add evm disassembly support. #3595

Conversation

gogo2464 commented Jun 21, 2023

gogo2464 commented Jun 21, 2023

ret2libc left a comment

Choose a reason for hiding this comment

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

XVilka commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

gogo2464 commented Jun 23, 2023

Choose a reason for hiding this comment

gogo2464 commented Jul 19, 2023

gogo2464 commented Jul 19, 2023

gogo2464 commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wargio left a comment

Choose a reason for hiding this comment

gogo2464 commented Jul 28, 2023

gogo2464 commented Aug 21, 2023

XVilka commented Jan 11, 2024

gogo2464 commented Jan 12, 2024

wargio commented Jan 12, 2024