Merge lp:~jtaylor/ubuntu/lucid/gajim/multiple-CVE into lp:ubuntu/lucid/gajim

[Julian Taylor]

[lp:~jtaylor/ubuntu/lucid/gajim/multiple-CVE]

54. By Julian Taylor on 2012-05-10

fix missing jid tuple in patch

55. By Julian Taylor on 2012-05-10

fix missing wait on process end

[Marc Deslauriers (mdeslaur)]

Julian, could you please update the status of this merge request so it gets removed from the sponsors list? Thanks.

=== modified file 'debian/changelog'
--- debian/changelog	2009-12-15 02:32:45 +0000
+++ debian/changelog	2012-05-10 19:54:17 +0000
@@ -1,3 +1,22 @@
+gajim (0.13-0ubuntu2.10.04.1) lucid-security; urgency=low
+
+  * SECURITY UPDATE: assisted code execution (LP: #992618)
+    - debian/patches/CVE-2012-2085.dpatch: fix subprocess call to prevent
+      shell escape from via crafted messages
+      https://trac.gajim.org/changeset/bc296e96ac10
+    - CVE-2012-2085
+  * SECURITY UPDATE: sql injection in logging code (LP: #992618)
+    - debian/patches/CVE-2012-2086.dpatch: use a prepated statement
+      https://trac.gajim.org/changeset/bfd5f94489d8
+    - CVE-2012-2086
+  * SECURITY UPDATE: insecure tmpfile creation (LP: #992613)
+    - debian/patches/CVE-2012-2093.dpatch: use safe tmpfile functions
+      when convering LaTeX IM messages to png images
+      Thanks to Nico Golde
+    - CVE-2012-2093
+
+ -- Julian Taylor <[e-mail]>  Tue, 01 May 2012 15:21:25 +0200
+
 gajim (0.13-0ubuntu2) lucid; urgency=low
 
   * debian/control:
 
=== modified file 'debian/patches/00list'
--- debian/patches/00list	2009-11-25 08:33:47 +0000
+++ debian/patches/00list	2012-05-10 19:54:17 +0000
@@ -1,2 +1,5 @@
 config-write-sync.patch
 ubuntu-keyring.patch
+CVE-2012-2085.dpatch
+CVE-2012-2086.dpatch
+CVE-2012-2093.dpatch
 
=== added file 'debian/patches/CVE-2012-2085.dpatch'
--- debian/patches/CVE-2012-2085.dpatch	1970-01-01 00:00:00 +0000
+++ debian/patches/CVE-2012-2085.dpatch	2012-05-10 19:54:17 +0000
@@ -0,0 +1,54 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## Description: prevent assisted code execution CVE-2012-2085
+## Origin: https://trac.gajim.org/changeset/bc296e96ac10
+## Bug: https://trac.gajim.org/ticket/7031
+## CVE-2012-2085.dpatch by Julian Taylor <[e-mail]>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: No description.
+
+@DPATCH@
+diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' gajim-lucid.orig~/src/common/helpers.py gajim-lucid.orig/src/common/helpers.py
+--- gajim-lucid.orig~/src/common/helpers.py	2012-05-01 15:19:52.000000000 +0200
++++ gajim-lucid.orig/src/common/helpers.py	2012-05-01 15:20:49.347118151 +0200
+@@ -39,6 +39,7 @@
+ import base64
+ import sys
+ import hashlib
++import shlex
+ 
+ from encodings.punycode import punycode_encode
+ 
+@@ -355,8 +356,18 @@
+ 			pass
+ 	return False
+ 
+-def exec_command(command):
+-	subprocess.Popen('%s &' % command, shell=True).wait()
++def exec_command(command, use_shell=False):
++	"""
++	execute a command. if use_shell is True, we run the command as is it was
++	typed in a console. So it may be dangerous if you are not sure about what
++	is executed.
++	"""
++	if use_shell:
++		subprocess.Popen('%s &' % command, shell=True).wait()
++	else:
++		args = shlex.split(command.encode('utf-8'))
++		p = subprocess.Popen(args)
++		gajim.thread_interface(p.wait)
+ 
+ def build_command(executable, parameter):
+ 	# we add to the parameter (can hold path with spaces)
+diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' gajim-lucid.orig~/src/notify.py gajim-lucid.orig/src/notify.py
+--- gajim-lucid.orig~/src/notify.py	2012-05-01 15:19:52.000000000 +0200
++++ gajim-lucid.orig/src/notify.py	2012-05-01 15:21:18.347117755 +0200
+@@ -323,7 +323,7 @@
+ 		command = gajim.config.get_per('notifications', str(advanced_notif_num),
+ 			'command')
+ 		try:
+-			helpers.exec_command(command)
++			helpers.exec_command(command, use_shell=True)
+ 		except Exception:
+ 			pass
+ 
 
=== added file 'debian/patches/CVE-2012-2086.dpatch'
--- debian/patches/CVE-2012-2086.dpatch	1970-01-01 00:00:00 +0000
+++ debian/patches/CVE-2012-2086.dpatch	2012-05-10 19:54:17 +0000
@@ -0,0 +1,157 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## Description: prevent sql injections CVE-2012-2086
+## Origin: https://trac.gajim.org/changeset/bc296e96ac10
+## Bug: https://trac.gajim.org/ticket/7031
+## CVE-2012-2086.dpatch by Julian Taylor <[e-mail]>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: No description.
+
+@DPATCH@
+diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' gajim-lucid.orig~/src/common/logger.py gajim-lucid.orig/src/common/logger.py
+--- gajim-lucid.orig~/src/common/logger.py	2012-05-01 15:19:52.000000000 +0200
++++ gajim-lucid.orig/src/common/logger.py	2012-05-01 15:23:03.891116311 +0200
+@@ -527,7 +527,7 @@
+ 		except exceptions.PysqliteOperationalError, e:
+ 			# Error trying to create a new jid_id. This means there is no log
+ 			return []
+-		where_sql = self._build_contact_where(account, jid)
++		where_sql, jid_tuple = self._build_contact_where(account, jid)
+ 
+ 		now = int(float(time.time()))
+ 		timed_out = now - (timeout * 60) # before that they are too old
+@@ -539,10 +539,9 @@
+ 				WHERE (%s) AND kind IN (%d, %d, %d, %d, %d) AND time > %d
+ 				ORDER BY time DESC LIMIT %d OFFSET %d
+ 				''' % (where_sql, constants.KIND_SINGLE_MSG_RECV,
+-					constants.KIND_CHAT_MSG_RECV, constants.KIND_SINGLE_MSG_SENT,
+-					constants.KIND_CHAT_MSG_SENT, constants.KIND_ERROR,
+-					timed_out, restore_how_many_rows, pending_how_many)
+-				)
++				constants.KIND_CHAT_MSG_RECV, constants.KIND_SINGLE_MSG_SENT,
++				constants.KIND_CHAT_MSG_SENT, constants.KIND_ERROR, timed_out,
++				restore_how_many_rows, pending_how_many), jid_tuple)
+ 
+ 			results = self.cur.fetchall()
+ 		except sqlite.DatabaseError:
+@@ -569,7 +568,7 @@
+ 		except exceptions.PysqliteOperationalError, e:
+ 			# Error trying to create a new jid_id. This means there is no log
+ 			return []
+-		where_sql = self._build_contact_where(account, jid)
++		where_sql, jid_tuple = self._build_contact_where(account, jid)
+ 
+ 		start_of_day = self.get_unix_time_from_date(year, month, day)
+ 		seconds_in_a_day = 86400 # 60 * 60 * 24
+@@ -580,7 +579,7 @@
+ 			WHERE (%s)
+ 			AND time BETWEEN %d AND %d
+ 			ORDER BY time
+-			''' % (where_sql, start_of_day, last_second_of_day))
++			''' % (where_sql, start_of_day, last_second_of_day), jid_tuple)
+ 
+ 		results = self.cur.fetchall()
+ 		return results
+@@ -603,13 +602,13 @@
+ 				return results
+ 
+ 		else: # user just typed something, we search in message column
+-			where_sql = self._build_contact_where(account, jid)
++			where_sql, jid_tuple = self._build_contact_where(account, jid)
+ 			like_sql = '%' + query.replace("'", "''") + '%'
+ 			self.cur.execute('''
+ 				SELECT contact_name, time, kind, show, message, subject FROM logs
+ 				WHERE (%s) AND message LIKE '%s'
+ 				ORDER BY time
+-				''' % (where_sql, like_sql))
++				''' % (where_sql, like_sql), jid_tuple)
+ 
+ 		results = self.cur.fetchall()
+ 		return results
+@@ -622,7 +621,7 @@
+ 			# Error trying to create a new jid_id. This means there is no log
+ 			return []
+ 		days_with_logs = []
+-		where_sql = self._build_contact_where(account, jid)
++		where_sql, jid_tuple = self._build_contact_where(account, jid)
+ 
+ 		# First select all date of month whith logs we want
+ 		start_of_month = self.get_unix_time_from_date(year, month, 1)
+@@ -640,7 +639,7 @@
+ 			AND kind NOT IN (%d, %d)
+ 			ORDER BY time
+ 			''' % (where_sql, start_of_month, last_second_of_month,
+-			constants.KIND_STATUS, constants.KIND_GCSTATUS))
++			constants.KIND_STATUS, constants.KIND_GCSTATUS), jid_tuple)
+ 		result = self.cur.fetchall()
+ 
+ 		# convert timestamps to day of month
+@@ -654,19 +653,21 @@
+ 		we had logs (excluding statuses)'''
+ 		where_sql = ''
+ 		if not is_room:
+-			where_sql = self._build_contact_where(account, jid)
++			where_sql, jid_tuple = self._build_contact_where(account, jid)
+ 		else:
+ 			try:
+ 				jid_id = self.get_jid_id(jid, 'ROOM')
+ 			except exceptions.PysqliteOperationalError, e:
+ 				# Error trying to create a new jid_id. This means there is no log
+ 				return None
+-			where_sql = 'jid_id = %s' % jid_id
++			where_sql = 'jid_id = ?'
++			jid_tuple = (jid_id,)
+ 		self.cur.execute('''
+ 			SELECT MAX(time) FROM logs
+ 			WHERE (%s)
+ 			AND kind NOT IN (%d, %d)
+-			''' % (where_sql, constants.KIND_STATUS, constants.KIND_GCSTATUS))
++			''' % (where_sql, constants.KIND_STATUS, constants.KIND_GCSTATUS),
++			jid_tuple)
+ 
+ 		results = self.cur.fetchone()
+ 		if results is not None:
+@@ -683,11 +684,13 @@
+ 		except exceptions.PysqliteOperationalError, e:
+ 			# Error trying to create a new jid_id. This means there is no log
+ 			return None
+-		where_sql = 'jid_id = %s' % jid_id
++		where_sql = 'jid_id = ?' 
++		jid_tuple = (jid_id,) 
++		
+ 		self.cur.execute('''
+ 			SELECT time FROM rooms_last_message_time
+ 			WHERE (%s)
+-			''' % (where_sql))
++			''' % (where_sql), jid_tuple)
+ 
+ 		results = self.cur.fetchone()
+ 		if results is not None:
+@@ -709,6 +712,7 @@
+ 		'''build the where clause for a jid, including metacontacts
+ 		jid(s) if any'''
+ 		where_sql = ''
++		jid_tuple = ()
+ 		# will return empty list if jid is not associated with
+ 		# any metacontacts
+ 		family = gajim.contacts.get_metacontacts_family(account, jid)
+@@ -718,13 +722,15 @@
+ 					jid_id = self.get_jid_id(user['jid'])
+ 				except exceptions.PysqliteOperationalError, e:
+ 					continue
+-				where_sql += 'jid_id = %s' % jid_id
++				where_sql += 'jid_id = ?'
++				jid_tuple += (jid_id,)
+ 				if user != family[-1]:
+ 					where_sql += ' OR '
+ 		else: # if jid was not associated with metacontacts
+ 			jid_id = self.get_jid_id(jid)
+-			where_sql = 'jid_id = %s' % jid_id
+-		return where_sql
++			where_sql = 'jid_id = ?'
++			jid_tuple += (jid_id,)
++		return where_sql,jid_tuple
+ 
+ 	def save_transport_type(self, jid, type_):
+ 		'''save the type of the transport in DB'''
+
 
=== added file 'debian/patches/CVE-2012-2093.dpatch'
--- debian/patches/CVE-2012-2093.dpatch	1970-01-01 00:00:00 +0000
+++ debian/patches/CVE-2012-2093.dpatch	2012-05-10 19:54:17 +0000
@@ -0,0 +1,114 @@
+#! /bin/sh /usr/share/dpatch/dpatch-run
+## Description: fix insecure tmpfile creation CVE-2012-2093
+## Origin: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668710
+## Bug: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=668710
+## CVE-2012-2093.dpatch by Julian Taylor <[e-mail]>
+##
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: No description.
+
+@DPATCH@
+diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' gajim-lucid.orig~/src/common/latex.py gajim-lucid.orig/src/common/latex.py
+--- gajim-lucid.orig~/src/common/latex.py	2012-05-01 15:19:52.000000000 +0200
++++ gajim-lucid.orig/src/common/latex.py	2012-05-01 15:26:22.031113594 +0200
+@@ -29,7 +29,7 @@
+ 
+ import os
+ import random
+-from tempfile import gettempdir
++from tempfile import gettempdir,mkstemp,mkdtemp
+ from subprocess import Popen, PIPE
+ 
+ import logging
+@@ -57,10 +57,10 @@
+ 			return True
+ 	return False
+ 
+-def get_tmpfile_name():
++def get_tmpfile_name(tmpdir):
+ 	random.seed()
+ 	int_ = random.randint(0, 100)
+-	return os.path.join(gettempdir(), 'gajimtex_' + int_.__str__())
++	return os.path.join(tmpdir, 'gajimtex_' + int_.__str__())
+ 
+ def write_latex(filename, str_):
+ 	texstr = '\\documentclass[12pt]{article}\\usepackage[dvips]{graphicx}'
+@@ -78,12 +78,12 @@
+ # a wrapper for Popen so that no window gets opened on Windows
+ # (i think this is the reason we're using Popen rather than just system())
+ # stdout goes to a pipe so that it can be read
+-def popen_nt_friendly(command):
++def popen_nt_friendly(command, directory):
+ 	if os.name == 'nt':
+ 		# CREATE_NO_WINDOW
+-		return Popen(command, creationflags=0x08000000, cwd=gettempdir(), stdout=PIPE)
++		return Popen(command, creationflags=0x08000000, cwd=directory, stdout=PIPE)
+ 	else:
+-		return Popen(command, cwd=gettempdir(), stdout=PIPE)
++		return Popen(command, cwd=directory, stdout=PIPE)
+ 
+ def check_for_latex_support():
+ 	'''check is latex is available and if it can create a picture.'''
+@@ -98,9 +98,9 @@
+ 	except LatexError:
+ 		return False
+ 
+-def try_run(argv):
++def try_run(argv, directory):
+ 	try:
+-		p = popen_nt_friendly(argv)
++		p = popen_nt_friendly(argv, directory)
+ 		out = p.communicate()[0]
+ 		log.info(out)
+ 		return p.wait()
+@@ -125,21 +125,28 @@
+ 		# we triggered the blacklist, immediately return None
+ 		return None
+ 
+-	tmpfile = get_tmpfile_name()
++	tmpdir = ""
++	tmppng = ""
++	try:
++		tmpdir = mkdtemp(prefix="gajim")
++		tmppng = mkstemp(suffix=".png")[1]
++	except Exception:
++		raise LatexError("could not securely create one or more temporary files for LaTeX conversion")
+ 
++	tmpfile = get_tmpfile_name(tmpdir)
+ 	# build latex string
+ 	write_latex(os.path.join(tmpfile + '.tex'), str_)
+ 
+ 	# convert TeX to dvi
+ 	exitcode = try_run(['latex', '--interaction=nonstopmode',
+-			  tmpfile + '.tex'])
++			  tmpfile + '.tex'], tmpdir)
+ 
+ 	if exitcode == 0:
+ 		# convert dvi to png
+ 		latex_png_dpi = gajim.config.get('latex_png_dpi')
+ 		exitcode = try_run(['dvipng', '-bg', bg_str, '-fg', fg_str, '-T',
+ 				'tight', '-D', latex_png_dpi, tmpfile + '.dvi', '-o',
+-				tmpfile + '.png'])
++				tmpfile + '.png'], tmpdir)
+ 
+ 	# remove temp files created by us and TeX
+ 	extensions = ['.tex', '.log', '.aux', '.dvi']
+@@ -149,11 +156,17 @@
+ 		except Exception:
+ 			pass
+ 
++	if exitcode == 0:
++		os.rename(tmpfile + '.png', tmppng)
++	else:
++		os.remove(tmppng)
++
++	os.rmdir(tmpdir)
+ 	if isinstance(exitcode, (unicode, str)):
+ 		raise LatexError(exitcode)
+ 
+ 	if exitcode == 0:
+-		result = tmpfile + '.png'
++		result = tmppng
+ 
+ 	return result
+