From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: taylanbayirli@gmail.com (Taylan Ulrich =?UTF-8?Q?Bay=C4=B1rl=C4=B1/Kammer?=) Newsgroups: gmane.emacs.bugs Subject: bug#21702: shell-quote-argument semantics and safety Date: Mon, 19 Oct 2015 09:34:15 +0200 Message-ID: <87h9lnpb0o.fsf@T420.taylan> References: <871tcstkuk.fsf@T420.taylan> <83pp0chzax.fsf@gnu.org> <874mhoq9ct.fsf@T420.taylan> <83h9lohsao.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1445240127 13526 80.91.229.3 (19 Oct 2015 07:35:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 19 Oct 2015 07:35:27 +0000 (UTC) Cc: 21702@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Oct 19 09:35:13 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Zo4yP-0006Le-0Q for geb-bug-gnu-emacs@m.gmane.org; Mon, 19 Oct 2015 09:35:13 +0200 Original-Received: from localhost ([::1]:36986 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zo4yO-00080u-5V for geb-bug-gnu-emacs@m.gmane.org; Mon, 19 Oct 2015 03:35:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zo4yJ-0007x1-Eg for bug-gnu-emacs@gnu.org; Mon, 19 Oct 2015 03:35:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zo4yF-0004D8-C0 for bug-gnu-emacs@gnu.org; Mon, 19 Oct 2015 03:35:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:36726) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zo4yF-0004D1-8W for bug-gnu-emacs@gnu.org; Mon, 19 Oct 2015 03:35:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1Zo4yE-00061T-SA for bug-gnu-emacs@gnu.org; Mon, 19 Oct 2015 03:35:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: taylanbayirli@gmail.com (Taylan Ulrich =?UTF-8?Q?Bay=C4=B1rl=C4=B1/Kammer?=) Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 19 Oct 2015 07:35:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 21702 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 21702-submit@debbugs.gnu.org id=B21702.144524006223103 (code B ref 21702); Mon, 19 Oct 2015 07:35:02 +0000 Original-Received: (at 21702) by debbugs.gnu.org; 19 Oct 2015 07:34:22 +0000 Original-Received: from localhost ([127.0.0.1]:55667 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Zo4xZ-00060W-HY for submit@debbugs.gnu.org; Mon, 19 Oct 2015 03:34:22 -0400 Original-Received: from mail-wi0-f171.google.com ([209.85.212.171]:34919) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Zo4xW-00060L-E6 for 21702@debbugs.gnu.org; Mon, 19 Oct 2015 03:34:19 -0400 Original-Received: by wicll6 with SMTP id ll6so82791913wic.0 for <21702@debbugs.gnu.org>; Mon, 19 Oct 2015 00:34:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-type:content-transfer-encoding; bh=RaITETtAoEVLjXrrfgBmgUNbGs9NHUbFrlUXrI8cy9s=; b=QKej1Bu777yLbrft3mdkDFJneSW8DxXlNc+CJyjMUCOV8HZbKkpDbThELQbE1R30eA hPKMOVGiU5ETQvFrHLvvbRPJ8a/4wtb2YFQrkBzp1q8PA537xXRlmXdFbNnXx6jAFeHN GEsZsEFDD4ExNlDKG2yHlsVG8csFYBY9WwzTTn94QcXveF6rp1GVl7PJbDQtESYFs75p YBmY6XuQ6G9NGvj/78fTdx6jdRanKb/1wI1HC6E8JauFkJM5NHYLTwbtdFdoV3c+J2WS wqOkqJ2QiARKu2xSKOox5ZAYUDd4Gs/jLTZp4wVAjEYXvK/YeU5GXtZPGhFreJzrXjLN JzxA== X-Received: by 10.180.211.176 with SMTP id nd16mr19573351wic.83.1445240057551; Mon, 19 Oct 2015 00:34:17 -0700 (PDT) Original-Received: from T420.taylan ([2a02:908:c32:4740:221:ccff:fe66:68f0]) by smtp.gmail.com with ESMTPSA id bk4sm38368531wjc.1.2015.10.19.00.34.15 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 19 Oct 2015 00:34:16 -0700 (PDT) In-Reply-To: <83h9lohsao.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 18 Oct 2015 22:48:15 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:107738 Archived-At: Eli Zaretskii writes: >> From: taylanbayirli@gmail.com (Taylan Ulrich Bay=C4=B1rl=C4=B1/Kammer) >> Cc: 21702@debbugs.gnu.org >> Date: Sun, 18 Oct 2015 21:12:34 +0200 >>=20 >> I'd like to point out that (in the most extreme cases) people have >> actually been writing web servers and other such programs in Elisp for >> which one would normally use a general-purpose language. >>=20 >> That is, "APIs that could be maliciously abused" is not the right way to >> look at it. It's not about the Elisp programmer abusing the API, it's >> about a malicious data source exploiting a (potential) flaw in an Elisp >> function, which Elisp programmers have relied on and thus made their >> programs vulnerable to code injection. >>=20 >>=20 >> That's why I was being so careful with regard to the safety guarantees >> of the "shell-quasiquote" package I contributed. I would like people to >> be able to use that as part of a general-purpose Elisp language, and so >> being safe against code injection is an absolute must. They might after >> all use it as part of a network-facing service. >>=20 >>=20 >> Actually that might also apply when using e.g. TRAMP, which also >> communicates with remote hosts and is a normal part of Emacs. I've been >> told it receives file names from remote hosts and passes them through >> shell-quote-argument before giving them to a shell. So maybe my >> concerns apply there as well. >>=20 >>=20 >> Given that, "I think 1) is now covered" is not very relieving to hear. > > Item 1 was this: > >> >> The function should clearly document >> >>=20 >> >> 1) for which shells will the quoting work absolutely, i.e. lead to >> >> the given string to appear *verbatim* in an element of the ARGV of >> >> the called command, > > There's nothing about safety here, only about correctness. That is > the aspect that I think is now covered, as the doc string now says for > which shells one can have correct results. Usually it's indeed correctness that protects against injection attacks. A quoting mechanism that's correct is automatically safe. Another way to make it safe would be to error when the given string contains characters outside of a limited character set. Either way, the safeness should be documented clearly, either implicitly through a clear documentation of the correctness, or explicitly. In your patch, correctness is implied, but the complexity of the problem domain (and thus the function itself) and the importance of possible repercussions of an incorrect implementation leave clearer documentation to be desired. While any function is really implied to be correct by its existence, any function is also implied to very possibly contain bugs, as is natural for software. In many cases these bugs are unimportant. In this case not. I would propose something along the lines of: It is guaranteed that ARGUMENT will be parsed as a single token by shells X, Y, and Z, as long as it is separated from other text via a delimiter in the syntax of the respective shell. (That's even better than the patch I proposed, which didn't mention the problem of delimiting.) I think it's also important to provide some explicit enumeration of shells for which the function is safe, because the systems Emacs supports may change over time, and there is no guarantee that a change to this function will not entail bugs. If we add wording like the above, then any programmer who sits down to expand the function's semantics to another shell will be forced to think very hard about what they're doing; otherwise they might try to do a "good enough" job but not make sure that all edge-cases are handled. "Designed to work with" is after all not an absolute claim of correctness and absence of bugs. >> It amounts to "I think this is safe against code injection" which is >> rather alarming to hear. Either it's very confidently known to be safe >> and so one may use it for network-facing code, or it's not confidently >> known to be safe and so one shouldn't use it for network-facing code. >> This should be documented clearly especially so that users who aren't >> very aware of injection attacks won't nonchalantly use the function for >> their network-facing code (when the function isn't known to be safe for >> this), but also so that users who are aware of such issues know they can >> use the function and don't instead invent their own thing (when it is >> known to be safe). >>=20 >> Does that make sense? > > Maybe it does, but only if we start documenting these aspects > project-wide. It makes little sense to me to do that for a single > API, and not an important one at that. But that's me. This is an API which if its implementation is imperfect will result in programs prone to code injection attacks when these programs face untrusted input sources. Why do you say it's not an important one? Taylan