Re: Bash bind bug: infinite loop with memory leak in Readline

[Chet Ramey]

On 9/22/16 11:23 AM, Christian Klomp wrote:

> Limiting the nesting level sounds like a straightforward solution.
> Although I'm not sure whether exposing a configurable level is
> necessary. Do people actually nest their macros that deep that no
> reasonably safe upper bound can be set?

There's no reason to expose the level and allow it to be configured.
I just meant capping execution at some maximum nesting level.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: Bash bind bug: infinite loop with memory leak in Readline

[Christian Klomp]

2016-09-19 3:41 GMT+02:00 Chet Ramey :
> Yes, you've triggered an infinite loop with the key binding.  One of the
> strengths of macros is that the expansion is not simply a string -- it can
> be used as shorthand for a complex key sequence, so simply disallowing
> the general case is not really an option.  I don't think anyone is going
> to deliberately bind a key to itself, so I'm not sure putting code in to
> solve that specific case, while leaving the general case unsolved, is
> worthwhile.
>
> Maybe the thing to do is to abort at some maximum macro nesting level, sort
> of like bash does with $FUNCNEST.
>

Yes, I agree that my code is too generic and the case a bit
far-fetched. There indeed appear to be more issues. For instance
binding "loop" to "looper" will also trigger it. Still quite unlikely
but more in the direction of something a user might expect to work.

Limiting the nesting level sounds like a straightforward solution.
Although I'm not sure whether exposing a configurable level is
necessary. Do people actually nest their macros that deep that no
reasonably safe upper bound can be set?

Re: Bash bind bug: infinite loop with memory leak in Readline

[Chet Ramey]

On 8/10/16 11:42 AM, Christian Klomp wrote:
> Hi,
> 
> I found a problem with the binding of key sequences to macros that
> results in bash allocating all the memory until the process is killed.
> It happens when a key is mapped to itself, e.g., `bind '"3":"3"'`. Now
> when '3' is typed the while loop inside the readline_internal_charloop
> function will never end.

Yes, you've triggered an infinite loop with the key binding.  One of the
strengths of macros is that the expansion is not simply a string -- it can
be used as shorthand for a complex key sequence, so simply disallowing
the general case is not really an option.  I don't think anyone is going
to deliberately bind a key to itself, so I'm not sure putting code in to
solve that specific case, while leaving the general case unsolved, is
worthwhile.

Maybe the thing to do is to abort at some maximum macro nesting level, sort
of like bash does with $FUNCNEST.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Bash bind bug: infinite loop with memory leak in Readline

[Christian Klomp]

Hi,

I found a problem with the binding of key sequences to macros that
results in bash allocating all the memory until the process is killed.
It happens when a key is mapped to itself, e.g., `bind '"3":"3"'`. Now
when '3' is typed the while loop inside the readline_internal_charloop
function will never end.

Besides manually, the flaw can be triggered via programs like expect,
screen and tmux, and works over SSH. When scripted it can result in
significant resource usage which might ultimately result in a denial
of service (although impact can be considered low since users need to
be authenticated and mitigation should be fairly easy, e.g., via
ulimit).

As far as I can tell every distribution and every Bash version is
affected by this (oldest I tried is Damn Small Linux 0.4.10 with Bash
2.05b) and it also happens on the new Windows Ubuntu subsystem.

I've written a preliminary patch that prevents triggering the flaw.
The first part prevents adding problematic macros and the second part
will escape the loop if a problematic key mapping occurs (I'm not sure
this second part is necessary assuming the first part properly
prevents adding any problematic mapping, but I've included it since
I'm not sure whether there are other ways to add mappings). So the
underlying problem is not addressed by the patch.

It also doesn't catch every case, e.g., `bind '"3":"123"'` will still
result in an infinite loop but in that case accumulation of memory is
insignificant.

Since I am not known with the internals of Bash/Readline and I am not
a C programmer I've refrained myself from digging further.

Regards,
Christian Klomp


# Bash 4.3.46:
--- ../bash-4.3/lib/readline/bind.c2013-04-06 23:46:38.0 +0200
+++ lib/readline/bind.c2016-08-09 21:55:56.500772211 +0200
@@ -313,6 +313,14 @@
  const char *keyseq, *macro;
  Keymap map;
 {
+  /* don't add macro if input is the same as output
+ in order to avoid infinite loop. */
+  if (*keyseq == *macro)
+{
+  _rl_init_file_error ("keyseq cannot be the same as macro");
+  return -1;
+}
+
   char *macro_keys;
   int macro_keys_len;

--- ../bash-4.3/lib/readline/readline.c2016-08-09 17:14:28.0 +0200
+++ lib/readline/readline.c2016-08-09 21:55:08.596984135 +0200
@@ -957,6 +957,14 @@
 {
   rl_executing_keyseq[rl_key_sequence_length] = '\0';
   macro = savestring ((char *)map[key].function);
+
+  /* avoid infinite loop if key and macro are equal. */
+  if (key == *macro)
+{
+  _rl_abort_internal ();
+  return -1;
+}
+
   _rl_with_macro_input (macro);
   return 0;
 }


# Bash 4.4-beta-2 (only line numbers differ):
--- ../bash-4.4-beta2/lib/readline/bind.c2016-01-25 16:33:57.0 +0100
+++ lib/readline/bind.c2016-08-09 22:06:42.602407370 +0200
@@ -337,6 +337,14 @@
  const char *keyseq, *macro;
  Keymap map;
 {
+  /* don't add macro if input is the same as output
+ in order to avoid infinite loop. */
+  if (*keyseq == *macro)
+{
+  _rl_init_file_error ("keyseq cannot be the same as macro");
+  return -1;
+}
+
   char *macro_keys;
   int macro_keys_len;

--- ../bash-4.4-beta2/lib/readline/readline.c2016-04-20
21:53:52.0 +0200
+++ lib/readline/readline.c2016-08-09 22:07:58.824249663 +0200
@@ -996,6 +996,14 @@
 {
   rl_executing_keyseq[rl_key_sequence_length] = '\0';
   macro = savestring ((char *)map[key].function);
+
+  /* avoid infinite loop if key and macro are equal. */
+  if (key == *macro)
+{
+  _rl_abort_internal ();
+  return -1;
+}
+
   _rl_with_macro_input (macro);
   return 0;
 }