Re: Escape sequences in log messages [etc]

[Daniel Shahaf]

Nathan Hartman wrote on Wed, Jan 18, 2023 at 01:10:47 -0500:
> On Tue, Jan 17, 2023 at 3:02 PM Doug Robinson 
> wrote:
> 
> > Daniel, et. al.:
> >
> > On Mon, Jan 2, 2023 at 5:14 PM Daniel Sahlberg <
> > daniel.l.sahlb...@gmail.com> wrote:
> >
> >> In a thread started by Vincent Lefevre in October [1] it was noted that
> >> Subversion prints several pieces of information from the repository to the
> >> terminal (including log messages and author names) without considering if
> >> they may affect terminal behaviour.
> >>
> >> As demonstrated by DanielSh [2] a user may inject escape sequences into a
> >> log message and when running svn log, these affect terminal color. Git
> >> behaves the same way, as demonstrated by me [3].
> >>
> >
> > Any idea what Git is going to do with this?
> >
> 
> 
> Unless someone reports (reported?) it to the Git devs, it's possible they
> aren't aware of it.
> 
> If we want to do something about it on our end, it might make sense to
> coordinate with the Git devs so that both systems could have similar
> behavior.
> 
> But... I'm not sure whether we want to do anything yet, partly because...
> 
> 
> Can we reach consensus if this behaviour is intended, unintended but
> >> desirable or unintended and undesirable? I would value the opinions of the
> >> oldtimers who might have background information if this was ever discussed
> >> or considered in the early days.
> >>
> >> In the original thread there were several arguments both pro and con
> >> regarding filtering/quoting escape sequences.
> >>
> >
> > From my perspective trying to do anything about this is opening up a huge
> > investigation that may result in incompatible-with-history choices.
> >
> > 1. What about "svn diff" ?  (any modifications here could break "patch",
> > et. al.)
> > 2. What about "svn cat" ?
> > 3. What about properties?  (I just verified you can place escape sequences
> > in them).
> > ...
> >
> > (I doubt my list above is complete.)
> >
> 
> 
> ...of concerns that doing so will break stuff.
> 

We have precedents for making breaking changes: we make them in an A.B.0
release if possible, and document them in the release notes and/or in
notes/api-errata/.

> > A "complete" implementation of a "feature" to mask/protect-against escape
> > sequences is also going to need an option to enable the raw output
> > (including the escape sequences) for every command/context where they could
> > be coming out today.

Not necessarily: If the lack of escaping may be considered a bugfix,
then we don't have to offer a backwards-compatible upgrade path.

The values of log messages are required to be UTF-8 strings.  When
ViewVC renders them, it escapes characters that are special to HTML
(angle brackets and ampersand).  If someone out there has a C source
code generator that takes svn log messages as input, that generator
should escape double quotes, backslashes, and so on when it emits C
string literals derived from log messages.  And when the cmdline client
emits svn:log property values to a terminal, it should escape the
sequences as appropriate for that terminal.

And we needn't support an option to emit escape sequences in raw form
for the same reason that ViewVC doesn't have an option to emit log
messages into the HTTP response stream without HTML escaping.

Cheers,

Daniel

Re: Escape sequences in log messages [etc]

[Nathan Hartman]

On Tue, Jan 17, 2023 at 3:02 PM Doug Robinson 
wrote:

> Daniel, et. al.:
>
> On Mon, Jan 2, 2023 at 5:14 PM Daniel Sahlberg <
> daniel.l.sahlb...@gmail.com> wrote:
>
>> In a thread started by Vincent Lefevre in October [1] it was noted that
>> Subversion prints several pieces of information from the repository to the
>> terminal (including log messages and author names) without considering if
>> they may affect terminal behaviour.
>>
>> As demonstrated by DanielSh [2] a user may inject escape sequences into a
>> log message and when running svn log, these affect terminal color. Git
>> behaves the same way, as demonstrated by me [3].
>>
>
> Any idea what Git is going to do with this?
>


Unless someone reports (reported?) it to the Git devs, it's possible they
aren't aware of it.

If we want to do something about it on our end, it might make sense to
coordinate with the Git devs so that both systems could have similar
behavior.

But... I'm not sure whether we want to do anything yet, partly because...


Can we reach consensus if this behaviour is intended, unintended but
>> desirable or unintended and undesirable? I would value the opinions of the
>> oldtimers who might have background information if this was ever discussed
>> or considered in the early days.
>>
>> In the original thread there were several arguments both pro and con
>> regarding filtering/quoting escape sequences.
>>
>
> From my perspective trying to do anything about this is opening up a huge
> investigation that may result in incompatible-with-history choices.
>
> 1. What about "svn diff" ?  (any modifications here could break "patch",
> et. al.)
> 2. What about "svn cat" ?
> 3. What about properties?  (I just verified you can place escape sequences
> in them).
> ...
>
> (I doubt my list above is complete.)
>


...of concerns that doing so will break stuff.

I have other concerns as well, such as: will we end up forever discovering
and having to add new cases of escape sequences?

I meant to research the above question but haven't gotten to it because of
other work. As part of that, I meant to look into what (if anything)
standards like POSIX have to say about whether and what programs should
filter/escape in their output, and how some of the standard Unix utilities
behave -- I can check this on Linux and Mac; the latter uses BSD utilities,
which have slightly different behavior to the GNU utilities. It should be
quite informative to try both.


A "complete" implementation of a "feature" to mask/protect-against escape
> sequences is also going to need an option to enable the raw output
> (including the escape sequences) for every command/context where they could
> be coming out today.  The reason is that somebody out there has possibly
> used that "feature" as a "capability" to automate something.  Preventing
> the output now would completely break whatever automation they cooked up.
> And it is unlikely that anyone inheriting that automation will understand
> it enough AND be reading this thread to object.
>
> Changing the default behavior is also something that could be argued to be
> breaking change requiring a "major number" to get bumped (e.g. a new
> release - not a patch)...
>
> Just some thoughts.
>


These are all good thoughts. Thanks for sharing.

Cheers,
Nathan

Re: Escape sequences in log messages [etc]

[Doug Robinson]

Daniel, et. al.:

On Mon, Jan 2, 2023 at 5:14 PM Daniel Sahlberg 
wrote:

> In a thread started by Vincent Lefevre in October [1] it was noted that
> Subversion prints several pieces of information from the repository to the
> terminal (including log messages and author names) without considering if
> they may affect terminal behaviour.
>
> As demonstrated by DanielSh [2] a user may inject escape sequences into a
> log message and when running svn log, these affect terminal color. Git
> behaves the same way, as demonstrated by me [3].
>

Any idea what Git is going to do with this?


> Can we reach consensus if this behaviour is intended, unintended but
> desirable or unintended and undesirable? I would value the opinions of the
> oldtimers who might have background information if this was ever discussed
> or considered in the early days.
>
> In the original thread there were several arguments both pro and con
> regarding filtering/quoting escape sequences.
>

>From my perspective trying to do anything about this is opening up a huge
investigation that may result in incompatible-with-history choices.

1. What about "svn diff" ?  (any modifications here could break "patch",
et. al.)
2. What about "svn cat" ?
3. What about properties?  (I just verified you can place escape sequences
in them).
...

(I doubt my list above is complete.)

A "complete" implementation of a "feature" to mask/protect-against escape
sequences is also going to need an option to enable the raw output
(including the escape sequences) for every command/context where they could
be coming out today.  The reason is that somebody out there has possibly
used that "feature" as a "capability" to automate something.  Preventing
the output now would completely break whatever automation they cooked up.
And it is unlikely that anyone inheriting that automation will understand
it enough AND be reading this thread to object.

Changing the default behavior is also something that could be argued to be
breaking change requiring a "major number" to get bumped (e.g. a new
release - not a patch)...

Just some thoughts.

Doug

-- 


 


THIS MESSAGE AND ANY ATTACHMENTS ARE 
CONFIDENTIAL, PROPRIETARY AND MAY BE PRIVILEGED


If this message was 
misdirected, WANdisco, Inc. and its subsidiaries, ("WANdisco") does not 
waive any confidentiality or privilege. If you are not the intended 
recipient, please notify us immediately and destroy the message without 
disclosing its contents to anyone. Any distribution, use or copying of this 
email or the information it contains by other than an intended recipient is 
unauthorized. The views and opinions expressed in this email message are 
the author's own and may not reflect the views and opinions of WANdisco, 
unless the author is authorized by WANdisco to express such views or 
opinions on its behalf. All email sent to or from this address is subject 
to electronic storage and review by WANdisco. Although WANdisco operates 
anti-virus programs, it does not accept responsibility for any damage 
whatsoever caused by viruses being passed.

AW: Escape sequences in log messages [etc]

[Markus Schaber]

Hi, Daniel,

> In a thread started by Vincent Lefevre in October [1] it was noted that 
> Subversion prints several pieces of information from the repository to the 
> terminal (including log messages and author names) without considering if 
> they may affect terminal behavior.
> 
> As demonstrated by DanielSh [2] a user may inject escape sequences into a log 
> message and when running svn log, these affect terminal color. Git behaves 
> the same way, as demonstrated by me [3].
> 
> Can we reach consensus if this behavior is intended, unintended but desirable 
> or unintended and undesirable? I would value the opinions of the oldtimers 
> who might have background information if this was ever discussed or 
> considered in the early days.
> 
> In the original thread there were several arguments both pro and con 
> regarding filtering/quoting escape sequences.

Some escape sequences may be useful (e. G. changing color, or bold printing) to 
get formatted log messages. But others may be dangerous, ranging from making 
the display unreadable or manipulating the window title to command execution.

As I think SVN is not in the business of deep inspection of escape sequences to 
classify the security relevant ones, we could either use a small whitelist of 
possible sequences, or just filter out / mask any of them.

Permitting some escape sequences requires that we reset the state to a known 
good one after printing the log message / username etc.

Some tools like "less" sanitize and filter escape sequences in a quite 
sophisticated way - maybe that logic is available as a library we can use? If 
not, I'd either allow a small whitelist, or just genrally mask ESC (and maybe 
some other control characters) on output.

Btw, to "officially" support "formatted log messages" as a feature, escape 
sequences are a rather inaccessible way - better alternatives would be things 
like some kind of wiki-style markup, a subset of html, rtf or similar.

Just my thoughts as a past (very) small scale contributor.

Regards,
Markus Schaber

Escape sequences in log messages [etc]

[Daniel Sahlberg]

Hi,

In a thread started by Vincent Lefevre in October [1] it was noted that
Subversion prints several pieces of information from the repository to the
terminal (including log messages and author names) without considering if
they may affect terminal behaviour.

As demonstrated by DanielSh [2] a user may inject escape sequences into a
log message and when running svn log, these affect terminal color. Git
behaves the same way, as demonstrated by me [3].

Can we reach consensus if this behaviour is intended, unintended but
desirable or unintended and undesirable? I would value the opinions of the
oldtimers who might have background information if this was ever discussed
or considered in the early days.

In the original thread there were several arguments both pro and con
regarding filtering/quoting escape sequences.

Kind regards,
Daniel Sahlberg


[1] https://lists.apache.org/thread/1m0yhsgj6vnb15h23p9h689qp597fhgm
[2] https://lists.apache.org/thread/1sr8srjm0jdqm75p0n1scj01g0d97mns
[3] https://lists.apache.org/thread/kln8bvfqo8961tqfmpkkhf69cqljkxdl