Trojan Source detection/highlight in Emacs?

Trojan Source detection/highlight in Emacs?

[Skip Montanaro]

The recent Trojan Source vulnerability crossed my newsfeed a day or two
ago. Here's an article from Krebs on Security:

https://krebsonsecurity.com/2021/11/trojan-source-bug-threatens-the-security-of-all-code/

Here's the rub:

Most programming languages let you put these Bidi overrides in comments and
strings. This is bad because most programming languages allow comments
within which all text — including control characters — is ignored by
compilers and interpreters. Also, it’s bad because most programming
languages allow string literals that may contain arbitrary characters,
including control characters.

...

The research paper, which dubbed the vulnerability “Trojan Source,” notes
that while both comments and strings will have syntax-specific semantics
indicating their start and end, *these bounds are not respected by Bidi
overrides*.


Krebs didn't give a concrete code example, but did reference a Rust Lang
blog post which does:

https://blog.rust-lang.org/2021/11/01/cve-2021-42574.html

As an example, the following snippet (with {U+NNNN} replaced with the
Unicode codepoint NNNN):


if access_level != "user{U+202E} {U+2066}// Check if admin{U+2069}
{U+2066}" {


...would be rendered by bidirectional-aware tools as:


if access_level != "user" { // Check if admin


This would give the reader the mistaken impression that the program is
comparing admin_level with the value "user".

There is also a C example on the Trojan Source website (scroll down):

https://trojansource.codes/

You can also get to the PDF of the paper describing the problem.

Rust is adding detection to its lint tool. It seems that may be the
approach taken by the maintainers of other languages.

The Python community is working on a PEP for this (doesn't even yet have a
number), but you can view the nascent PEP and discussion here:

https://mail.python.org/archives/list/python-dev@python.org/thread/6DBJJRQHA2SP5Q27MOMDSTCOXMW7ITNR/#6DBJJRQHA2SP5Q27MOMDSTCOXMW7ITNR

IDEs, editors, and lint tools are probably where the bulk of the action
will be. Has this been discussed within the Emacs developer community?
Maybe a bidi minor mode would be a good place to implement some
colorization, with the minor mode enabled by default in most programming
language major modes (with easy disabling by the user).

Let's be careful out there...

Skip Montanaro

Re: Trojan Source detection/highlight in Emacs?

[Stefan Monnier via Users list for the GNU Emacs text editor]

> if access_level != "user{U+202E} {U+2066}// Check if admin{U+2069}
> {U+2066}" {
>
> ...would be rendered by bidirectional-aware tools as:
>
> if access_level != "user" { // Check if admin
>
> This would give the reader the mistaken impression that the program is
> comparing admin_level with the value "user".

Clearly, Eli will know better, but I suspect that we may be able to
avoid most of those issues by (conceptually) treating comment delimiters
as bidi barriers.  Of course, that leaves open the question of what
I mean by "bidi barrier" and of how to implement it ;-)


        Stefan

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> Date: Mon, 01 Nov 2021 19:25:55 -0400
> From:  Stefan Monnier via Users list for the GNU Emacs text editor <help-gnu-emacs@gnu.org>
> 
> > if access_level != "user{U+202E} {U+2066}// Check if admin{U+2069}
> > {U+2066}" {
> >
> > ...would be rendered by bidirectional-aware tools as:
> >
> > if access_level != "user" { // Check if admin
> >
> > This would give the reader the mistaken impression that the program is
> > comparing admin_level with the value "user".
> 
> Clearly, Eli will know better, but I suspect that we may be able to
> avoid most of those issues by (conceptually) treating comment delimiters
> as bidi barriers.  Of course, that leaves open the question of what
> I mean by "bidi barrier" and of how to implement it ;-)

It's more than that: bidi reordering happens on a very low level in
the display engine, where there's absolutely no information about
stuff like comment delimiters and PL syntax in general.  In
particular, that code runs before font-lock and similar features
examined the text syntactically and decided what is and what isn't a
comment.

So doing what you propose would be ... how should I say it? ... very
difficult.

We could instead provide an ability to bidi-reorder only certain
stretches of text, marked by some special text property.  Then a Lisp
program could mark only comments and strings with that property, and
the reordering would not happen anywhere else in the buffer.  Doing
something like that is relatively simple, but not too simple, so I'd
say this particular issue doesn't justify the effort.

I would start with detecting such reordered code and flagging it.  We
are almost there; all that's needed is a motivated individual to
implement the user-level feature using infrastructure that already
(almost) exists.

Re: Trojan Source detection/highlight in Emacs?

[Stefan Monnier via Users list for the GNU Emacs text editor]

>> Clearly, Eli will know better, but I suspect that we may be able to
>> avoid most of those issues by (conceptually) treating comment delimiters
>> as bidi barriers.  Of course, that leaves open the question of what
>> I mean by "bidi barrier" and of how to implement it ;-)
>
> It's more than that: bidi reordering happens on a very low level in
> the display engine, where there's absolutely no information about
> stuff like comment delimiters and PL syntax in general.  In
> particular, that code runs before font-lock and similar features
> examined the text syntactically and decided what is and what isn't a
> comment.

That's the "how to implement it" part, yes.  BTW, I don't think it's the
case that bidi reordering is done "before font-lock and similar features
examined the text" (bidi reordering applies to text rendering and
font-lock faces are applied before the buffer's text is rendered).

> We could instead provide an ability to bidi-reorder only certain
> stretches of text, marked by some special text property.  Then a Lisp
> program could mark only comments and strings with that property, and
> the reordering would not happen anywhere else in the buffer.

If we don't want to allow properly rendered Hebrew identifiers, then
it's indeed a great solution.  It would be pretty easy to make font-lock
add a special `bidi-enable` text property to strings and comments.

> Doing something like that is relatively simple, but not too simple, so
> I'd say this particular issue doesn't justify the effort.

I suspect none of the solutions are "too simple" to implement.
But I don't think we should rush to implement something, since there are
several ways to attack this problem, and it's probably worth thinking
a bit more about the full extent of the problem (after all, it's mostly
a problem of security, where we hence should assume the attacker will be
clever enough to try and circumvent the simple measures we may come up
with).

> I would start with detecting such reordered code and flagging it.

Indeed, another approach is to render it "normally" and then flag those
places where the rendering may mislead the reader, which could also
include the confusables.

A simple and straightforward way to do that is to highlight any
non-ASCII char, and to render all the "non printing" chars (such as
RIGHT-TO-LEFT OVERRIDE) as tofu or something like that (otherwise, the
highlighting applied to it wouldn't be visible).


        Stefan

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> Date: Tue, 02 Nov 2021 10:56:50 -0400
> From:  Stefan Monnier via Users list for the GNU Emacs text editor <help-gnu-emacs@gnu.org>
> 
> I don't think it's the case that bidi reordering is done "before
> font-lock and similar features examined the text" (bidi reordering
> applies to text rendering and font-lock faces are applied before the
> buffer's text is rendered).

font-lock is applied as part of redisplay, in layers that are above
bidi reordering.

> > I would start with detecting such reordered code and flagging it.
> 
> Indeed, another approach is to render it "normally" and then flag those
> places where the rendering may mislead the reader, which could also
> include the confusables.

Isn't that the same as I said?

> A simple and straightforward way to do that is to highlight any
> non-ASCII char, and to render all the "non printing" chars (such as
> RIGHT-TO-LEFT OVERRIDE) as tofu or something like that (otherwise, the
> highlighting applied to it wouldn't be visible).

That's already available, no changes needed.

Re: Trojan Source detection/highlight in Emacs?

[Stefan Monnier via Users list for the GNU Emacs text editor]

>> if access_level != "user{U+202E} {U+2066}// Check if admin{U+2069}
>> {U+2066}" {
>>
>> ...would be rendered by bidirectional-aware tools as:
>>
>> if access_level != "user" { // Check if admin
>>
>> This would give the reader the mistaken impression that the program is
>> comparing admin_level with the value "user".
>
> Clearly, Eli will know better, but I suspect that we may be able to
> avoid most of those issues by (conceptually) treating comment delimiters
> as bidi barriers.  Of course, that leaves open the question of what
> I mean by "bidi barrier" and of how to implement it ;-)

Tho, actually, the problem is more pronounced since bidi can also be
used within a string (as shown above already) as well as within
identifiers (tho language may opt to disallow them there).

So my suggestion above would have to be extended to treat string
delimiters as barriers, and similarly for "identifier delimiters"
(i.e. whitespace, infix operators/punctuation, ...) tho not when within
comments or strings.
And of course, the specifics are all language-dependent.
Clearly non-trivial.


        Stefan

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> From: Skip Montanaro <skip.montanaro@gmail.com>
> Date: Mon, 1 Nov 2021 17:19:16 -0500
> 
> The recent Trojan Source vulnerability crossed my newsfeed a day or two
> ago.

For some value of "recent".

> IDEs, editors, and lint tools are probably where the bulk of the action
> will be. Has this been discussed within the Emacs developer community?

We had a long discussion of a very similar issue, in the context of
URLs and phishing.  It started here:

  https://lists.gnu.org/archive/html/emacs-devel/2014-11/msg02203.html

and continued into the next month:

  https://lists.gnu.org/archive/html/emacs-devel/2014-12/msg00004.html

As result of these discussions, I implemented a special function,
bidi-find-overridden-directionality, which is part of Emacs since
version 25.1, released 5 years ago.  (Don't rush to invoke that
function with the code samples mentioned above: it won't catch them.)
My expectation, and the reason why I bothered to write that function,
was that given the interest and the long discussion, the function will
immediately be used in some URL-related code in Emacs.  That didn't
happen, and the function is collecting dust in Emacs ever since.

Now, the code there is not ready for the kind of tricks these new
examples are playing, so it doesn't detect them.  It can be enhanced
to do that, though.  But I'm reluctant to invest my time and energy in
a feature that will just keep collecting dust.  So I will only work on
this if someone is actually prepared to use this function in Emacs by
adding some user-facing UI features, like making the problematic text
stand out on display, or displaying a warning.

I should also mention that Emacs has (weak) defenses against this kind
of tricks: we show the formatting control characters on display,
unlike other editors that hide them.  Also, cursor motion with C-f and
C-b will seem to behave erratically if you move across the problematic
text.  So users that actually look at the code they use will most
probably find out that something strange is going on (if they don't
look, no visual cue will do).

Re: Trojan Source detection/highlight in Emacs?

[Skip Montanaro]

On Tue, Nov 2, 2021 at 9:39 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Skip Montanaro <skip.montanaro@gmail.com>
> > Date: Mon, 1 Nov 2021 17:19:16 -0500
> >
> > The recent Trojan Source vulnerability crossed my newsfeed a day or two
> > ago.
>
> For some value of "recent".
>

:-)

It's not clear when the paper was published, but I expect Brian Krebs to be
on top of things better than most people. That this is only now becoming an
issue in the software development space suggests that back in 2014, it
wasn't viewed as a potentially broad vulnerability by the Emacs developer
community at the time.

I accept your belief that this will be difficult to handle in Emacs. Maybe
lint tools are where the solution lies.

Skip

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> From: Skip Montanaro <skip.montanaro@gmail.com>
> Date: Tue, 2 Nov 2021 10:01:28 -0500
> Cc: Help GNU Emacs <help-gnu-emacs@gnu.org>
> 
> I accept your belief that this will be difficult to handle in Emacs. Maybe lint tools are where the solution lies.

I didn't say I thought it would be hard.  I invited motivated
individuals to use the existing infrastructure to detect and flag
such text as part of the usual Emacs display of code.

Re: Trojan Source detection/highlight in Emacs?

[Stefan Monnier via Users list for the GNU Emacs text editor]

> Now, the code there is not ready for the kind of tricks these new
> examples are playing, so it doesn't detect them.  It can be enhanced
> to do that, though.  But I'm reluctant to invest my time and energy in
> a feature that will just keep collecting dust.  So I will only work on
> this if someone is actually prepared to use this function in Emacs by
> adding some user-facing UI features, like making the problematic text
> stand out on display, or displaying a warning.

I can see two use cases:

- One that's disabled by default, and where we can expect the users to
  accept a fairly strict definition of "normal" (e.g. flag any non-ASCII
  char as suspicious).  That should be pretty easy to implement, but
  very rarely used (only by security-conscious people who happen to
  work almost exclusively with code using English identifiers and
  comments).

- One that's enabled by default, but in that case it'll have to be a lot
  more permissive so as not to get in the way of people who want to
  write comments and identifiers in their mother tongue.  Making this
  permissive enough without leaving gaping security holes seems hard.

> I should also mention that Emacs has (weak) defenses against this kind
> of tricks: we show the formatting control characters on display,
> unlike other editors that hide them.  Also, cursor motion with C-f and
> C-b will seem to behave erratically if you move across the problematic
> text.  So users that actually look at the code they use will most
> probably find out that something strange is going on (if they don't
> look, no visual cue will do).

If the readers are only reviewing the code without actually editing it,
there's a significant probability that they won't move across the
problematic case with the cursor (they'll only do that with their eyes).


        Stefan

Re: Trojan Source detection/highlight in Emacs?

[Anders Munch]

Eli Zaretskii wrote:
> Stefan Monnier wrote: 
>> A simple and straightforward way to do that is to highlight any 
>> non-ASCII char, and to render all the "non printing" chars (such as 
>> RIGHT-TO-LEFT OVERRIDE) as tofu or something like that (otherwise, the 
>> highlighting applied to it wouldn't be visible).
>
> That's already available, no changes needed.

Can we get a recipe, please?
For the "non-printing" chars part that is - I certainly wouldn't want to ruin all non-ASCII text, homoglyphs be damned.

I tried customising bidi-paragraph-direction, setting it to 'left-to-right, but I'm not seeing any effect, despite the docstring saying it /forces/ directionality. I guess it just sets a default, and explicit bidi control characters take precedence.
 
regards, Anders

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> From: Anders Munch <ajm@flonidan.dk>
> Date: Wed, 3 Nov 2021 08:52:52 +0000
> Accept-Language: en-US
> 
> Eli Zaretskii wrote:
> > Stefan Monnier wrote: 
> >> A simple and straightforward way to do that is to highlight any 
> >> non-ASCII char, and to render all the "non printing" chars (such as 
> >> RIGHT-TO-LEFT OVERRIDE) as tofu or something like that (otherwise, the 
> >> highlighting applied to it wouldn't be visible).
> >
> > That's already available, no changes needed.
> 
> Can we get a recipe, please?

Customize the variable glyphless-char-display-control.

> I tried customising bidi-paragraph-direction, setting it to 'left-to-right, but I'm not seeing any effect, despite the docstring saying it /forces/ directionality.

The doc string says it forces the directionality of the paragraph.  If
you don't already know what that means, I suggest to read the
"Bidirectional Editing" node in the Emacs manual, it should explain
that.

> I guess it just sets a default, and explicit bidi control characters take precedence.

Not exactly, but something like that.

The paragraph direction is not relevant to this issue, since in
prog-mode descendants we already set the value to left-to-right.

Re: Trojan Source detection/highlight in Emacs?

[Anders Munch]

Eli Zaretskii wrote:
> Anders Munch <ajm@flonidan.dk> wrote:
>> Can we get a recipe, please?
>
> Customize the variable glyphless-char-display-control.

Thanks for the pointer, that would be the 'format-control' group?

I see it set to "Display as thin space".
But that's only half the story. The other half of how bidi control characters display is the effect that they have on surrounding text. 
glyphless-char-display-control does not affect that.

>  The doc string says it [bidi-paragraph-direction] forces the directionality of the paragraph.  If you don't already know what that means, I suggest to read the "Bidirectional Editing" node in the Emacs manual, it should explain that.

I went looking for a bidi off switch precisely because I'm aware that the Unicode bidi rules are complicated, or at least confusing when put into practice, and for those of us that don't read RTL languages, it might be better to switch it off completely than to try to understand it.

bidi-paragraph-direction was the only candidate to a bidi off switch that I could find.
If that's not what it is, then where is the master switch to turn bidi processing off?

regards, Anders

Re: Trojan Source detection/highlight in Emacs?

[Eli Zaretskii]

> From: Anders Munch <ajm@flonidan.dk>
> Date: Wed, 3 Nov 2021 15:17:32 +0000
> 
> Eli Zaretskii wrote:
> > Anders Munch <ajm@flonidan.dk> wrote:
> >> Can we get a recipe, please?
> >
> > Customize the variable glyphless-char-display-control.
> 
> Thanks for the pointer, that would be the 'format-control' group?

Yes.

> I see it set to "Display as thin space".

Yes.

> But that's only half the story. The other half of how bidi control characters display is the effect that they have on surrounding text. 
> glyphless-char-display-control does not affect that.

That you cannot change easily, and you shouldn't try.  Emacs
implements the Unicode Bidirectional Algorithm when it produces
character glyphs for display, and that includes reordering them into
the so-called "visual" order.

> I went looking for a bidi off switch precisely because I'm aware that the Unicode bidi rules are complicated, or at least confusing when put into practice, and for those of us that don't read RTL languages, it might be better to switch it off completely than to try to understand it.
> 
> bidi-paragraph-direction was the only candidate to a bidi off switch that I could find.
> If that's not what it is, then where is the master switch to turn bidi processing off?

There is no such switch in Emacs.  Bidi reordering is always turned
on.  (And don't believe people who will tell you about
bidi-display-reordering: that variable doesn't disable the reordering
completely, exists only for my own debugging, and if turned off, will
cause the display code work unreliably in some situations.)