blinksh / blink

Blink Mobile Shell for iOS (Mosh based)
https://blink.sh
GNU General Public License v3.0
6.08k stars 565 forks source link

Hard crash entering `ctrl-^` then `.` then `enter` into `blink>` prompt #1994

Closed fingolfin closed 6 days ago

fingolfin commented 3 months ago

To exit a hanging mosh session, one should enter ctrl-^ . On my keyboard that is ctrl-shift-6 then . -- for various reasons I am in the habit of pressing enter after this, then repeating this 1-2 times (to deal with various lag issues etc.).

That lead me to discover that if I am on the blink> prompt and do this there (ctrl-^ then . then enter) it crashes Blink hard -- it just is dead, and reopening it afterwards various sessions I had closed magically re-appear (broken). In fact it seems all my connections are broken after this hard crash.

Actually I just tried, and just ctrl-6 followed by enter (or return) also triggers the crash.

This is with v17.2.2.868 on iOS 17.4 on an iPad Pro (11 inch, 3rd gen)

carloscabanero commented 3 months ago

That's an interesting one. Will take a look at what may be provoking the crash.

l2dy commented 3 months ago

I thought people won't type weird things and closed #1900. I was wrong.

Screenshot 2024-03-20

Edit: #1900 is not actually relevant, because this is valid UTF-8. The correspondence comes from "weird things typed".

fingolfin commented 3 months ago

Well I did not really input it intentionally, it just happened ;-). Still would be good if that didn't cause a hard crash

l2dy commented 3 months ago

No worries. I didn't mean to be harsh about this. I hold the same opinion that a crash or freeze should not happen.

This crash happens inside ios_system and I can reproduce it in a-shell too. I'm having a hard time getting Address Sanitizer to work inside ios_system, but there is some progress.

l2dy commented 3 months ago

Passing the string 1E 00 (Ctrl-^) to getLastCharacterOfArgument() returns an invalid pointer 0x1. This needs to be fixed upstream.

Screenshot 2024-03-21

P.S. Address Sanitizer was not needed for this.

l2dy commented 3 months ago

@carloscabanero I have a patch for it. It's very unfortunate that ios_system uses ^^ as its internal Record Separator for arguments with both ' and ". The internal ^^ always comes in pairs, except when you put one in the input ;)

diff --git a/ios_system.m b/ios_system.m
index 21e9eeb..87a498b 100644
--- a/ios_system.m
+++ b/ios_system.m
@@ -2459,6 +2459,7 @@ static char* getLastCharacterOfArgument(const char* argument) {
         return NULL;
     } else if (argument[0] == recordSeparator) {
         char* endquote = strchr(argument + 1, recordSeparator);
+        if (endquote == NULL) return NULL; // be safe
         return endquote + 1;
     }
     // TODO: the last character of the argument could also be '<' or '>' (vim does that, with no space after file name)
carloscabanero commented 6 days ago

Done 17.3.0 . Thanks for the help