emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Add shell-quasiquote.


From: Random832
Subject: Re: [PATCH] Add shell-quasiquote.
Date: Sun, 18 Oct 2015 08:59:32 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Michael Albinus <address@hidden> writes:
> PS: I'm working as Security Consultant, and so I am paranoid per
> definition. But I'm not *such* paranoid until I see there are good
> reasons for.

I do think it's disappointing that people are having such a cavalier
attitude about this...

The documentation does say:

| Precisely what this function does depends on your operating
| system. The function is designed to work with the syntax of your
| system’s standard shell; if you use an unusual shell, you will
| need to redefine this function.

But it doesn't bother explaining what operating systems it works on,
what is an unusual shell, or that _not_ having it defined in a way
consistent with the shell has security implications.

I think this has contributed to Taylan having a "gut feeling" that
it may not be secure on Windows, because it is difficult to
understand the implementation and is not well-documented and the
attitude is not a good sign. For example, ^-quoting is only applied
if [%!"] are present, but is applied to [%!()"<>&|^]. Why? Who
knows? The linked documentation for CommandLineToArgV provides no
insight about this second level of quoting. Why does ms-dos have
separate logic from nt?

And I know there's nothing to be done for it, but the fact that it
does not have any way to escape wildcards is concerning. I think it
would be reasonable for it to be an error if a character that it
doesn't know how to handle or can't handle is present, rather than
just muddle through. The whole point of having a function is to get
it right; if you don't care about that then (format "command \"%s\""
filename) is good enough for 95% of usage.


Speaking of Tramp, what if the local shell is not the same as the
remote shell? And I don't see how the commands it runs "require a
bournish shell" at all. they require that the commands themselves
exist, but that's nothing to do with the shell.

Tramp also (as of Emacs 24.5) wraps shell-quote-argument in its own
logic which fixes a newline handling bug that is no longer present.
Which also violates the "don't reinvent the wheel" policy - the fix
should have been submitted to shell-quote-argument itself (as it
ultimately was), and should never have been included in a version of
tramp that shipped with Emacs.

It even has a TODO item:

;; * Rewrite `tramp-shell-quote-argument' to abstain from using
;;   `shell-quote-argument'.

So much for not reinventing the wheel.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]