Fix #16210 - Show error message and update help for `we` ##io #16427

radare · 2020-04-05T18:51:07Z

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 added tests that prove my fix is effective or that my feature works (if possible)
I've updated the documentation and the radare2 book with the relevant information (if needed)

Detailed description

just read the

Test plan

modifying in disk files is a bit tricky and boring, not going to add tests now for this we have more unit tests to add before this imho

Closing issues

the #16210 one

ret2libc · 2020-04-06T07:38:23Z

libr/core/cmd.c

@@ -2884,7 +2884,7 @@ static int r_core_cmd_subst_i(RCore *core, char *cmd, char *colon, bool *tmpseek
 			line = strdup (cmd);
 			line = r_str_replace (line, "\\\"", "\"", true);
 			if (p && *p && p[1] == '|') {
-				str = r_str_trim_head_ro (p + 2);
+				str = (char *)r_str_trim_head_ro (p + 2);


mmm i think you already did this change in another PR.

yep i think so too. but its not, otherwise i would get a conflict

ret2libc

Did you try the reproduce in the issue? It does not seem to work, I think. That makes sense, though, as your PR does not really change anything apart from reporting an error when r_core_extend_at returns false.

ret2libc · 2020-04-06T07:56:09Z

modifying in disk files is a bit tricky and boring, not going to add tests now for this we have more unit tests to add before this imho

I would add a test for this as well. It does not work.

radare · 2020-04-06T23:22:27Z

the wen command works fine:

$ echo hello world > txt
$ r2 -w txt
 -- I did it for the pwnz.
[0x00000000]> x 32
- offset -  0001 0203 0405 0607 0809 0A0B 0C0D 0E0F  0123456789ABCDEF
0x00000000  6865 6c6c 6f20 776f 726c 640a ffff ffff  hello world.....
0x00000010  ffff ffff ffff ffff ffff ffff ffff ffff  ................
[0x00000000]> s 3
[0x00000003]> wen 3
[0x00000003]> s 0
[0x00000000]> x 32
- offset -  0001 0203 0405 0607 0809 0A0B 0C0D 0E0F  0123456789ABCDEF
0x00000000  6865 6c00 0000 6c6f 2077 6f72 6c64 0aff  hel...lo world..
0x00000010  ffff ffff ffff ffff ffff ffff ffff ffff  ................
[0x00000000]> r
15
[0x00000000]> q
$ ls -l txt
-rw-r--r--  1 pancake  staff  15 Apr  7 01:21 txt
$ cat txt
hello world
$ hexdump -C txt|head -n 4
00000000  68 65 6c 00 00 00 6c 6f  20 77 6f 72 6c 64 0a     |hel...lo world.|
0000000f
$

radare · 2020-04-06T23:33:31Z

only works when io.va=false

radare · 2020-04-07T09:23:53Z

I can improve the help on that to specify that this only makes sense when the underlying io plugin can be resized and it's in PA mode

ret2libc · 2020-04-07T09:25:48Z

only works when io.va=false
I can improve the help on that to specify that this only makes sense when the underlying io plugin can be resized and it's in PA mode

That is ok, but please remove the issue #16210 from the title/commits/description, because this is not a fix for that issue. Then, if you think #16210 is notabug, close it by explaining your point of view.

If for now wen works only when io.va=false, it should be definitely explained in the help msg.

radare · 2020-04-07T22:04:59Z

$ r2 /tmp/ls
 -- This binary no good. Try another.
[0x10000102c]> x 32
- offset -   2C2D 2E2F 3031 3233 3435 3637 3839 3A3B  CDEF0123456789AB
0x10000102c  5548 89e5 4157 4156 4155 4154 5348 81ec  UH..AWAVAUATSH..
0x10000103c  1806 0000 4989 f741 89fe 488d 85c0 fdff  ....I..A..H.....
[0x10000102c]> wen 3
r_io_extend failed
[0x10000102c]> e io.va
true
[0x10000102c]> oo+
[0x10000102c]> x 3
- offset -   2C2D 2E2F 3031 3233 3435 3637 3839 3A3B  CDEF0123456789AB
0x10000102c  5548 89                                  UH.
[0x10000102c]> wen 3
works
[0x10000102c]> x
- offset -   2C2D 2E2F 3031 3233 3435 3637 3839 3A3B  CDEF0123456789AB
0x10000102c  0000 0055 4889 e541 5741 5641 5541 5453  ...UH..AWAVAUATS
0x10000103c  4881 ec18 0600 0049 89f7 4189 fe48 8d85  H......I..A..H..
0x10000104c  c0fd ffff 4889 45d0 85ff 7f05 e858 3100  ....H.E......X1.

now works better, i need to add more tests probably

lgtm-com · 2020-04-07T22:53:54Z

This pull request fixes 1 alert when merging c283836 into 3a95531 - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

ret2libc

If now it gives a proper error, that's much better!

ret2libc · 2020-04-08T08:31:39Z

libr/core/cmd_write.c

-					r_core_block_read (core);
+					if (cmd_suc) {
+						core->offset = cur_off;
+eprintf ("works\n");


ret2libc · 2020-04-08T08:32:55Z

libr/core/cio.c

 		}
+		r_config_set_i (core->config, "io.va", io_va);


why do you set io_va, considering it was never changed since you get it?

radare · 2020-04-08T10:37:45Z

i need to add a test where i modify a binary in VA mode, @thestr4ng3r how do you recommend to do taht after the last changes in r2r.c ? do i copy the file into tmp inside r2 and then reopen, and rm when done?

thestr4ng3r · 2020-04-08T10:41:08Z

Yes, see a88ec62#diff-f67ce03030de4a37ff248a555a9bb36c

ret2libc

Also, please remember to leave a comment in the original issue saying that it is not supported (at least for now) to use wen when io.va=false

radare · 2020-04-08T10:43:04Z

wat? @ret2libc wen works on io.va=true and false now. it just requires a writable and resizable underlying fd

ret2libc · 2020-04-08T10:45:16Z

wat? @ret2libc wen works on io.va=true and false now. it just requires a writable and resizable underlying fd

🤦‍♂️ right, sorry :D

radare · 2020-04-08T10:58:35Z

added another test

radare · 2020-04-08T10:58:50Z

ill squash+merge when ci ends

lgtm-com · 2020-04-08T11:34:29Z

This pull request fixes 1 alert when merging 45894c1 into f9864ef - view on LGTM.com

fixed alerts:

1 for Comparison result is always the same

radareorg#16427) * Fix wen command for io.va=true, add two tests

radare requested a review from ret2libc April 5, 2020 18:52

github-actions bot added the command New commands requests, behaviour changes, removal label Apr 5, 2020

radare added this to the 4.4.0 - pangolin milestone Apr 6, 2020

ret2libc reviewed Apr 6, 2020

View reviewed changes

ret2libc suggested changes Apr 6, 2020

View reviewed changes

radare requested a review from ret2libc April 7, 2020 09:16

radare added 2 commits April 8, 2020 00:04

Fix #16210 - Show error message and update help for we ##io

39b676b

Fix wen command for io.va=true, add two tests

c283836

radare force-pushed the fix-16210 branch from 08bec0c to c283836 Compare April 7, 2020 22:04

ret2libc suggested changes Apr 8, 2020

View reviewed changes

Fix io.va problem and address review comments

b0a0b13

radare requested review from ret2libc and thestr4ng3r April 8, 2020 10:37

ret2libc approved these changes Apr 8, 2020

View reviewed changes

Add one iova test

45894c1

radare merged commit a52506a into master Apr 8, 2020

Emi1305 pushed a commit to Emi1305/radare2 that referenced this pull request Jul 12, 2020

Fix radareorg#16210 - Show error message and update help for we ##io (

2b416eb

radareorg#16427) * Fix wen command for io.va=true, add two tests

ret2libc deleted the fix-16210 branch September 2, 2020 08:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #16210 - Show error message and update help for `we` ##io #16427

Fix #16210 - Show error message and update help for `we` ##io #16427

radare commented Apr 5, 2020 •

edited

Loading

ret2libc Apr 6, 2020

radare Apr 7, 2020

ret2libc left a comment

ret2libc commented Apr 6, 2020

radare commented Apr 6, 2020

radare commented Apr 6, 2020

radare commented Apr 7, 2020

ret2libc commented Apr 7, 2020

radare commented Apr 7, 2020

lgtm-com bot commented Apr 7, 2020

ret2libc left a comment

ret2libc Apr 8, 2020

ret2libc Apr 8, 2020

radare commented Apr 8, 2020

thestr4ng3r commented Apr 8, 2020

ret2libc left a comment

radare commented Apr 8, 2020

ret2libc commented Apr 8, 2020

radare commented Apr 8, 2020

radare commented Apr 8, 2020

lgtm-com bot commented Apr 8, 2020

Fix #16210 - Show error message and update help for we ##io #16427

Fix #16210 - Show error message and update help for we ##io #16427

Conversation

radare commented Apr 5, 2020 • edited Loading

ret2libc Apr 6, 2020

Choose a reason for hiding this comment

radare Apr 7, 2020

Choose a reason for hiding this comment

ret2libc left a comment

Choose a reason for hiding this comment

ret2libc commented Apr 6, 2020

radare commented Apr 6, 2020

radare commented Apr 6, 2020

radare commented Apr 7, 2020

ret2libc commented Apr 7, 2020

radare commented Apr 7, 2020

lgtm-com bot commented Apr 7, 2020

ret2libc left a comment

Choose a reason for hiding this comment

ret2libc Apr 8, 2020

Choose a reason for hiding this comment

ret2libc Apr 8, 2020

Choose a reason for hiding this comment

radare commented Apr 8, 2020

thestr4ng3r commented Apr 8, 2020

ret2libc left a comment

Choose a reason for hiding this comment

radare commented Apr 8, 2020

ret2libc commented Apr 8, 2020

radare commented Apr 8, 2020

radare commented Apr 8, 2020

lgtm-com bot commented Apr 8, 2020

Fix #16210 - Show error message and update help for `we` ##io #16427

Fix #16210 - Show error message and update help for `we` ##io #16427

radare commented Apr 5, 2020 •

edited

Loading