Project

General

Profile

Actions

Feature #18367

closed

Stop the interpreter from escaping error messages

Added by mame (Yusuke Endoh) 6 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:106308]

Description

Proposal

At the present time, the Ruby interpreter escapes some characters (*1) in error messages when an uncaught error is printed. I'd like to propose stopping this escaping behavior.

class MyError < StandardError
  def message
    "foo\\bar"
  end
end

raise MyError
#=> current:  test.rb:7: in `<main>': foo\\bar (MyError)
#=> excepted: test.rb:7: in `<main>': foo\bar (MyError)

*1: Escaped characters are any control characters except \t and \n, and a backslash \\.

Motivation

This behavior prevents us from adding an attribution (color, underline, etc.) to the error message because it escapes escape sequences. Nowadays, such a rich presentation of terminal output is more and more important.

$ ruby -e 'raise "\e[31mRed\x1b[0m error"'
-e:1:in `<main>': \e[31mRed\x1b[0m error (RuntimeError)

Also, the behavior in question leads to rather confusing error printing. See the error output of "\\".no_method:

$ ruby -e '"\\".no_method'
-e:1:in `<main>': undefined method `no_method' for "\\\\":String (NoMethodError)

"\\\\".no_method
    ^^^^^^^^^^

The two occurrences of "\\\\" must be "\\". Worse, the output of error_highlight ^^^^ points wrong position.

Note that this issue is never specific to error_highlight. The receiver of NoMethodError, "\\\\":String, is also wrongly escaped. It must be "\\":String.

Why the escaping behavior was introduced

AFAIK, the behavior was introduced because of a security concern. It is considered harmful for an attacker to be able to print arbitrary escape sequences to victim's terminal. (See this article in detail.)

However, I believe it is rare to see the error logs of an application that may be exposed to attacks (i.e. in production mode) in a terminal, as the error output of the Ruby interpreter.

Even if that is the case, I think such escaping should be done as a responsibility of the application, and not implicitly by the interpreter. I briefly surveyed other major languages than Ruby, and I could find no language that escapes error messages. This is the transcript of Python and Node.js.

$ python3 -c 'raise Exception("\x1b[31mRed\x1b[0m error")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
Exception: Red error

$ node -e 'throw("\x1b[31mRed\x1b[0m error")'

[eval]:1
throw("\x1b[31mRed\x1b[0m error")
^
Red error
(Use `node --trace-uncaught ...` to show where the exception was thrown)

Just in case, I reported these behaviors to the security contacts of Python and Node.js, and both responded to me that this is not a securty issue. I think their decisions are quite reasonable.

Migration

It would be a good idea to first make the following behavior as a migration path.

  • When an error message does not include a control character, no escaping is applied.
  • When an error message does include a control character, "Warning: this error message is currently escaped because it includes a control character(s), but this will not be escaped in Ruby 3.X" is printed, and the escaping is applied.

Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #18370: Call Exception#full_message to print exceptions reaching the top-levelClosedActions

Updated by sawa (Tsuyoshi Sawada) 6 months ago

+1

A very minor thing: "a control character(s)" is wrong. It should be either "a control character", "control characters", or "(a) control character(s)".

Updated by Eregon (Benoit Daloze) 6 months ago

:+1:
BTW, this is already the behavior in both JRuby and TruffleRuby :)

To clarify, does this affect only full_message and the default exception "printer"?

Updated by mame (Yusuke Endoh) 6 months ago

Eregon (Benoit Daloze) wrote in #note-2:

To clarify, does this affect only full_message and the default exception "printer"?

This affects only the exception "printer". full_message does not apply the escaping.

$ ruby -e 'begin; raise "\e[31mRed\x1b[0m error"; rescue Exception => exc; puts exc.full_message; end'
-e:1:in `<main>': Red error (RuntimeError)
Actions #6

Updated by Eregon (Benoit Daloze) 6 months ago

  • Related to Feature #18370: Call Exception#full_message to print exceptions reaching the top-level added

Updated by Dan0042 (Daniel DeLorme) 6 months ago

How is this supposed to interact with the default highlighting?

-e:1:in `': \e[31mRed\e[0m error (RuntimeError)

If the ANSI escape codes are not escaped, the \e[0m would reset the formatting early and it would look like

-e:1:in `': Red error (RuntimeError) (with colored "Red")

I'm not sure it makes sense to allow ANSI escapes that can only result in broken formatting.

What about just removing the backslash from the set of escaped characters? For most errors it can be misleading, as in the example of "\\".no_method above.

Updated by mame (Yusuke Endoh) 6 months ago

Dan0042 (Daniel DeLorme) wrote in #note-7:

I'm not sure it makes sense to allow ANSI escapes that can only result in broken formatting.

Good point. If you want to control attribution, I think you have to reset it by \e[0m at the beginning and control everything yourself.

What about just removing the backslash from the set of escaped characters? For most errors it can be misleading, as in the example of "\\".no_method above.

@nobu (Nobuyoshi Nakada) is against this because it becomes ambiguous.

Actions #9

Updated by mame (Yusuke Endoh) 6 months ago

  • Description updated (diff)

Updated by mame (Yusuke Endoh) 5 months ago

This ticket was discussed at the dev-meeting as a strongly-related topic of #18370.

We need to be careful to remove security measures. We should first check if the vulnerability in question is still valid with modern popular terminal emulators. At least, no change for Ruby 3.1.

I will survey terminals when I have time... (Or voluntery is really welcome.)

Updated by Eregon (Benoit Daloze) 5 months ago

IMHO if there is any vulnerability there it would be a bug of the terminal emulator, not Ruby.
And we'd have the same issue for any puts or logging when shown in a terminal without escaping.

If the attack is a bell sound or changing the terminal title it seems pretty harmless.

Updated by mame (Yusuke Endoh) 5 months ago

I read the article about the vulnerability again.

It clearly insists that this vulnerability should be addressed in the side of terminal emulators.

The responsibility
should rest on the actual terminal emulator; any features that allow file or command-line
access should be disabled by default and more attention should be paid to new features
that implement any use of escape sequences.

I did a quick survey of the current status of the two specific issues mentioned in the article.

"Screen Dumping" issue

Now rxvt requires "insecure mode" to enable the feature in question.

https://github.com/cpjreynolds/urxvt/blob/master/doc/rxvt.1.pod

insecure: boolean
Enables "insecure" mode. Rxvt-unicode offers some escape sequences that echo arbitrary strings like the icon name or the locale. This could be abused if somebody gets 8-bit-clean access to your display, whether through a mail client displaying mail bodies unfiltered or through write(1) or any other means. Therefore, these sequences are disabled by default. (Note that many other terminals, including xterm, have these sequences enabled by default, which doesn't make it safer, though).

You can enable them by setting this boolean resource or specifying -insecure as an option. At the moment, this enables display-answer, locale, findfont, icon label and window title requests.

I checked the behavior with rxvt-unicode (urxvt) v9.22.

With regard to Eterm, the feature in question seems to be disabled by #if 0 in 2002:

https://github.com/mej/Eterm/commit/c9ef7c900204b8e1ca980e02790d6bb45d4aa784

Mon Apr 29 10:48:33 2002 Michael Jennings (mej)
Disable the screen dump escape sequence and implement a save_buff()
script function instead.

"Window Title Reporting" issue

Now XTerm requires "allowWindowOps" configuration to enable the feature in question.
VTE (gnome-terminal) is aware of the issue and implements the feature as dummy (reports an empty dummy title).

        case VTE_XTERM_WM_GET_WINDOW_TITLE:
                /* Report a static window title, since the real
                 * window title should NEVER be reported, as it
                 * creates a security vulnerability.  See
                 * http://marc.info/?l=bugtraq&m=104612710031920&w=2
                 * and CVE-2003-0070.
                 */

I also checked Windows Terminal and iterm2. I couldn't find documentation about the feature, but as far as I tested, both do not report window title by the escape sequence in question.

Conclusion

In modern times, I think a consensus has already been reached that this kind of security problems should be solved on the terminal emulator side.

If anyone finds a modern terminal emulator still vulnerable, tell me (or the author of the terminal emulator :-).

Updated by usa (Usaku NAKAMURA) 4 months ago

BTW, if the target output is not terminal (istty = false), what kind of output is desirable?

Updated by mame (Yusuke Endoh) 4 months ago

usa (Usaku NAKAMURA) wrote in #note-13:

BTW, if the target output is not terminal (istty = false), what kind of output is desirable?

I don't think that has much to do with this proposal, but I basically expect the same output.

At the dev-meeting, @matz (Yukihiro Matsumoto) accepted this proposal. I'll improve and merge my pull request later.

Actions #15

Updated by mame (Yusuke Endoh) 3 months ago

  • Status changed from Open to Closed

Applied in changeset git|f207f7a193dc4e55820e77388edefb5d8fde18d7.


Do not escape error message

[Feature #18367]

Actions

Also available in: Atom PDF