[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    Re: [oss-security] Re: CVE request: lsyncd command injection
From:       Sven Schwedas <sven.schwedas () tao ! at>
Date:       2014-11-26 12:22:43
Message-ID: 5475C613.1010701 () tao ! at
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


On 2014-11-26 00:18, Ángel González wrote:
> On 20-11-2014 Mitre wrote:
>>> There is a command injection flaw in lsyncd, a file change monitoring
>>> and synchronization daemon:
>>>
>>> https://github.com/axkibe/lsyncd/issues/220
>>>
>>> https://github.com/creshal/lsyncd/commit/18f02ad013b41a72753912155ae2ba72f2a53e52
>>>
>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=767227
>>
>> Use CVE-2014-8990. The scope of this CVE ID includes both:
>>
>>   1. code execution with ` characters or other characters that are
>>      special to a shell
>>   2. denial of service scenarios in which a user with write access
>>      to a local directory uses special characters to make
>>      synchronization fail (might have security relevance in some
>>      scenarios)
>>
>> The MITRE CVE team does not have a Lua expert. The code change adds:
>>
>>   local path1 = event.path:gsub ('"', '\\"'):gsub ('`', '\\`'):gsub ('%$','\\%$')
>>   local path2 = event2.path:gsub ('"', '\\"'):gsub ('`', '\\`'):gsub ('%$','\\%$')
>>
>> This does not seem to be the typical fix approach for unsafe input to
>> a shell. Has anyone concluded that this is an incomplete fix that ought
>> to be modified before the 2.1.6 release?
> 
> 
> It is indeed an incomplete fix:
> 
> * The gsub ('%$','\\%$') works in lua5.1, but under lua5.2 the second %
> character makes lsyncd fail with the error "stdin:1: invalid use of '%'
> in replacement string". Thus allowing a complete denial of service
> 
> 
> * Not all metacharacters are filtered, so command execution is still
> present. In particular, the escaped characters can be prefixed with a
> backslash to bypass the filter.
> 
> 
> The attached patch should hopefully solve these issues.

Thank you. I've tested the patch locally and it appears to be working
correctly (mine was more a quick hack to get our own lsyncd instances
running again).
It also has been merged upstream:

> https://github.com/axkibe/lsyncd/commit/e9ffda07f0145f50f2756f8ee3fb0775b455122b

Attached is the patch adapted for Wheezy's lsyncd 2.0.7-3.

-- 
Mit freundlichen Grüßen, / Best Regards,
Sven Schwedas
Systemadministrator
TAO Beratungs- und Management GmbH | Lendplatz 45 | A - 8020 Graz
Mail/XMPP: sven.schwedas@tao.at | +43 (0)680 301 7167
http://software.tao.at

["0001-Properly-sanitize-mv-parameters-CVE-2014-8990.patch" (text/x-patch)]

From cb2cdea8ceff561dc10f41c17df00f74a8a9419e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81ngel=20Gonz=C3=A1lez?= <angel@16bits.net>
Date: Tue, 25 Nov 2014 23:49:25 +0100
Subject: [PATCH] Properly sanitize mv parameters (CVE-2014-8990)

When using -rsyncssh option, some filenames
could -in addition of not syncing correctly-
crash the service and execute arbitrary commands
under the credentials of the remote user.

These issues have been assigned CVE-2014-8990

This commit fixes the incomplete and lua5.2-incompatible
sanitization performed by 18f02ad0
---
 default-rsyncssh.lua | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/default-rsyncssh.lua b/default-rsyncssh.lua
index 589837d..3f3261d 100644
--- a/default-rsyncssh.lua
+++ b/default-rsyncssh.lua
@@ -77,8 +77,10 @@ rsyncssh.action = function( inlet )
 	-- makes move local on target host
 	-- if the move fails, it deletes the source
 	if event.etype == 'Move' then
-		local path1 = event.path:gsub ('"', '\\"'):gsub ('`', '\\`'):gsub ('%$','\\%$')
-		local path2 = event2.path:gsub ('"', '\\"'):gsub ('`', '\\`'):gsub ('%$','\\%$')
+		local path1 = config.targetdir .. event.path
+		local path2 = config.targetdir .. event2.path
+		path1 = "'" .. path1:gsub ('\'', '\'"\'"\'') .. "'"
+		path2 = "'" .. path2:gsub ('\'', '\'"\'"\'') .. "'"
 
 		log(
 			'Normal',
@@ -94,10 +96,10 @@ rsyncssh.action = function( inlet )
 			config.ssh._computed,
 			config.host,
 			'mv',
-			'\"' .. config.targetdir .. path1 .. '\"',
-			'\"' .. config.targetdir .. path2 .. '\"',
+			path1,
+			path2,
 			'||', 'rm', '-rf',
-			'\"' .. config.targetdir .. path1 .. '\"'
+			path1
 		)
 
 		return
-- 
2.1.3


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]