magiblot / tvision

A modern port of Turbo Vision 2.0, the classical framework for text-based user interfaces. Now cross-platform and with Unicode support.
Other
2k stars 151 forks source link

Ctrl+Space / Ctrl+@ (NUL) in input line on Linux eventually causes tvision crash #37

Closed akovalenko closed 3 years ago

akovalenko commented 3 years ago

Run tvedit, File->Open.., enter several Ctrl+@ characters (shown as spaces..), then backspace them. See tvedit crashes.

Fix might be like this:

From 90660811ec90279e1cec23df4fd186fc94a52b48 Mon Sep 17 00:00:00 2001
From: Anton Kovalenko <anton@sw4me.com>
Date: Mon, 21 Dec 2020 15:53:48 +0300
Subject: [PATCH] TInputLine: never add \NUL to an input line

strlen(data) is used on input line text, so putting \nul there
(e.g. pressing Ctrl+Space or Ctrl+@ on Linux) confuses TInputLine into
invalid memory access.
---
 source/tvision/tinputli.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source/tvision/tinputli.cpp b/source/tvision/tinputli.cpp
index f1569b7..bc1f8e1 100644
--- a/source/tvision/tinputli.cpp
+++ b/source/tvision/tinputli.cpp
@@ -356,7 +356,7 @@ void TInputLine::handleEvent( TEvent& event )
                         setState(sfCursorIns, Boolean(!(state & sfCursorIns)));
                         break;
                     default:
-                        if ((len = event.keyDown.textLength))
+                        if ((len = event.keyDown.textLength) && event.keyDown.text[0])
                             {
                             deleteSelect();
                             if( (state & sfCursorIns) != 0 )
-- 
2.27.0
magiblot commented 3 years ago

Hi Anton. Thanks for noticing this horrendous bug! It basically gives you free rein to overwrite the heap with zeros.

I totally overlooked the case of appending a non-null-terminated string to a null-terminated one. I'll add it to my list of bad things that may happen when dealing with null-terminated strings. It only keeps growing...

event.keyDown.text is not a null-terminated string, so in theory it may contain null characters at any position, not just the first one. So a more sophisticated solution is needed -- I'll work on it. Thank you for the patch anyway.