Howdy all,
The package ‘burn’ has a security bug open, assigned the alert number
TEMP-0542329 “burn: Insecure escaping of file names”. I have been
advised to make a bug-fix release of this package for ‘stable’ and
send a ‘debdiff’ output to this forum.
Please excuse my caution, since this is my first time making a
security update to a package in ‘stable’. I have a few questions about
whether I have followed procedures correctly.
The package for ‘stable’ is a non-maintainer upload, ‘0.4.3-2.2’,
since I was not the maintainer for the package prior to the release of
Lenny.
Question: Is this the correct way to make such a security update?
I have also set the changelog entry header with ‘urgency=high’, for
addressing a security alert.
Question: Is this the correct urgency level?
For this security update in ‘stable’, I have made changes that will
*not* be incorporated into the normal history of the package; e.g. the
‘debian/changelog’ entry for this release will not appear in the
Squeeze version of the package.
Question: Is it correct to put changes in a stable update that
effectively make a dead-end branch in the history?
Since I am not a Debian Developer, I have put the package online at
<URL:http://mentors.debian.net/debian/pool/main/b/burn/burn_0.4.3-2.2.dsc>.
The output of ‘debdiff burn_0.4.3-{2.1,2.2}.dsc’ is attached to this
message.
Question: Do I also need to separately seek a sponsor for this
package to be uploaded to Debian?
--
\ “You are welcome to visit the cemetery where famous Russian and |
`\ Soviet composers, artists, and writers are buried daily except |
_o__) Thursday.” —Russian orthodox monastery, Moscow |
Ben Finney <ben@benfinney.id.au>
diff -Nru burn-0.4.3/debian/changelog burn-0.4.3/debian/changelog
--- burn-0.4.3/debian/changelog 2009-08-23 15:12:43.000000000 +1000
+++ burn-0.4.3/debian/changelog 2009-08-23 15:44:01.000000000 +1000
@@ -1,3 +1,12 @@
+burn (0.4.3-2.2) stable; urgency=high
+
+ * Non-maintainer upload.
+ * Security fix for “TEMP-0542329 burn: Insecure escaping of file names”.
+ * Backport fix for secure handling of child process command arguments.
+ (Closes: Bug#542329)
+
+ -- Ben Finney <ben+debian@benfinney.id.au> Sun, 23 Aug 2009 15:43:10 +1000
+
burn (0.4.3-2.1) unstable; urgency=medium
* Non-maintainer upload.
diff -Nru burn-0.4.3/debian/control burn-0.4.3/debian/control
--- burn-0.4.3/debian/control 2009-08-23 15:12:43.000000000 +1000
+++ burn-0.4.3/debian/control 2009-08-23 15:44:01.000000000 +1000
@@ -2,7 +2,7 @@
Section: otherosfs
Priority: optional
Maintainer: Gaetano Paolone (bigpaul) <bigpaul@debian.org>
-Build-Depends: debhelper (>= 4.0.0)
+Build-Depends: debhelper (>= 4.0.0), quilt (>= 0.40)
Standards-Version: 3.6.0
Package: burn
diff -Nru burn-0.4.3/debian/patches/01.subprocess.patch burn-0.4.3/debian/patches/01.subprocess.patch
--- burn-0.4.3/debian/patches/01.subprocess.patch 1970-01-01 10:00:00.000000000 +1000
+++ burn-0.4.3/debian/patches/01.subprocess.patch 2009-08-23 15:44:01.000000000 +1000
@@ -0,0 +1,472 @@
+Author: Ben Finney <ben+debian@benfinney.id.au>
+Description: Bug#542329: burn: Quotation marks in filenames aren't handled properly.
+ Use ‘subprocess’ module, with command argument sanitisation, for
+ all invocation and interaction with child processes.
+
+=== modified file 'burn'
+--- old/burn 2005-03-16 19:37:40 +0000
++++ new/burn 2009-08-23 00:50:44 +0000
+@@ -42,9 +42,9 @@
+
+ from optparse import OptionParser, OptionGroup
+ from os.path import *
+-from os import system, remove, walk, listdir
++from os import remove, walk, listdir
+ from re import match, search, compile
+-import sys, ConfigParser, commands, glob, string
++import sys, ConfigParser, string
+ import os
+ import statvfs
+ import eyeD3
+@@ -52,7 +52,9 @@
+ import ao
+ import ogg.vorbis
+ import gettext
+-import popen2, pwd
++import pwd
++import subprocess
++import shlex
+ import tty, termios, time, fnmatch
+
+ gettext.bindtextdomain('burn', '/usr/share/locale/it/LC_MESSAGES/')
+@@ -147,12 +149,15 @@
+
+ def check_media_empty():
+ """check if cdrom is empty using cdrdao"""
+- # giuppi's function
+- device = config.get('CD-writer','device')
+- driver = config.get('CD-writer','driver')
+- media_raw_info=popen2.popen2('%s disk-info --device %s --driver %s 2>/dev/null'%(cdrdao,device,driver))
+- lines=media_raw_info[0].readlines()
+- for line in lines:
++ command_args = [
++ cdrdao, "disk-info",
++ "--device", config.get('CD-writer', 'device'),
++ "--driver", config.get('CD-writer', 'driver'),
++ ]
++ command_process = subprocess.Popen(
++ command_args, close_fds=True,
++ stdout=subprocess.PIPE, stderr=open(os.devnull))
++ for line in command_process.stdout:
+ if line.startswith('CD-R empty'):
+ if line.split(':')[1].strip()=='yes':
+ return True
+@@ -175,12 +180,15 @@
+
+ def get_media_capacity():
+ """get media capacity using cdrdao"""
+- # giuppi's function
+- device = config.get('CD-writer','device')
+- driver = config.get('CD-writer','driver')
+- media_raw_info=popen2.popen2('%s disk-info --device %s --driver %s 2>/dev/null'%(cdrdao,device,driver))
+- lines=media_raw_info[0].readlines()
+- for line in lines:
++ command_args = [
++ cdrdao, "disk-info",
++ "--device", config.get('CD-writer', 'device'),
++ "--driver", config.get('CD-writer', 'driver'),
++ ]
++ command_process = subprocess.Popen(
++ command_args, close_fds=True,
++ stdout=subprocess.PIPE, stderr=open(os.devnull))
++ for line in command_process.stdout:
+ if line.startswith('Total Capacity'):
+ if line.split(':')[1].strip()=='n/a':
+ return None
+@@ -284,38 +292,31 @@
+ class ISO:
+ "ISO class"
+ path_o = []
+- mkisofs_line = ''
++ mkisofs_args = []
+ windows = config.get('ISO','windows_read')
+ tempdir = config.get('ISO','tempdir')
+ image_name = config.get('ISO','image')
+ mount_dir = config.get('ISO','mount_dir')
+ dest = normpath(tempdir + image_name)
+- def mkisofs_line_append(self, addenda):
+- """append string (addenda) to mkisofs_line"""
+- self.mkisofs_line = self.mkisofs_line + addenda
+- def mkisofs_line_view(self):
+- """view command in mkisofs_line"""
+- return self.mkisofs_line
+ def create(self):
+- """exec command in mkisofs_line"""
++ """ Execute data track recording command. """
+ print _('Creating temporary image: '), self.dest
+ pbar=progressBar(0,100,30)
+- stderr=popen2.popen3(self.mkisofs_line)[2]
++ command_process = subprocess.Popen(
++ self.mkisofs_args, close_fds=True,
++ stderr=subprocess.PIPE)
+ progress=0
+- while 1:
+- line=stderr.readline()
+- if not line:
+- pbar.updateAmount(100)
+- print pbar, "\r",
+- sys.stdout.flush()
+- return
++ for line in command_process.stderr:
+ if "done, estimate finish" in line:
+ progress = int(float(line.split()[0][:-1]))
+ pbar.updateAmount(progress)
+ print pbar, "\r",
+ sys.stdout.flush()
++ pbar.updateAmount(100)
++ print pbar, "\r",
++ sys.stdout.flush()
+ def destroy(self):
+- """exec command in mkisofs_line"""
++ """ Remove the image file. """
+ remove(self.dest)
+ def ask_mount(self, image):
+ """asks if user wants to mount image"""
+@@ -330,16 +331,18 @@
+ def mount(self, image):
+ """mount image in self.mount_dir"""
+ if exists(self.mount_dir) and isdir(self.mount_dir):
+- mount_loop = "mount -o loop " + image + " " + self.mount_dir
+- if system(mount_loop):
++ command_args = [
++ "mount", "-o", "loop",
++ image, self.mount_dir]
++ if subprocess.call(command_args):
+ print _('Unable to mount '), image, _('. Please check if you have permissions to mount it on '), self.mount_dir
+ sys.exit()
+ self.ask_after_mount(self.mount_dir)
+ else:
+ err(self.mount_dir + _(' is not valid as a mount point'))
+ sys.exit()
+- umount_loop = "umount " + self.mount_dir
+- system(umount_loop)
++ command_args = ["umount", self.mount_dir]
++ subprocess.call(command_args)
+
+ def ask_after_mount(self, dirname):
+ """Choose what to do with the mounted image..."""
+@@ -396,7 +399,7 @@
+ class CDROM:
+ "CDROM class"
+
+- cdrecord_line = ''
++ cdrecord_args = []
+ cdrdao=config.get('executables','cdrdao')
+ speed = config.get('CD-writer','speed')
+ device = config.get('CD-writer','device')
+@@ -411,7 +414,15 @@
+ if not empty:
+ if not options.multisession:
+ print _('Error. Please insert a blank CD/DVD.')
+- os.system('%s -eject dev=%s &>/dev/null'%(cdrecord,config.get('CD-writer','device')))
++ device = config.get(
++ 'CD-writer','device')
++ command_args = [
++ cdrecord, "-eject",
++ "dev=%(device)s" % vars()]
++ subprocess.call(
++ command_args,
++ stdout=open(os.devnull),
++ stderr=open(os.devnull))
+ sys.exit()
+
+ self.size = get_media_capacity()
+@@ -421,15 +432,9 @@
+ else:
+ self.size = config.get('Media','size')
+
+- def cdrecord_line_append(self, addenda):
+- """appen string (addenda) to cdrecord_line"""
+- self.cdrecord_line = self.cdrecord_line + addenda
+ def cdrecord_simulate(self):
+ """simulate burning"""
+- self.cdrecord_line = self.cdrecord_line + '-dummy '
+- def cdrecord_line_view(self):
+- """view command in cdrecord_line"""
+- return self.cdrecord_line
++ self.cdrecord_args.append("-dummy")
+ def size_compare(self, tobeburned, space_needed):
+ """Checks free space for temporary files and CD oversize"""
+ free_disk_space = int(iso.freespace())
+@@ -456,62 +461,59 @@
+
+ def line_create(self):
+ """cdrecord line generation"""
+- self.cdrecord_line = ''
+- self.cdrecord_line_append(cdrecord + ' -v -pad ')
++ self.cdrecord_args = []
++ self.cdrecord_args.extend([cdrecord, "-v", "-pad"])
+ #check for dummy (simulate)
+ if options.simulate:
+ self.cdrecord_simulate()
+ #check for eject
+ if options.eject:
+- self.cdrecord_line_append('-eject ')
++ self.cdrecord_args.append("-eject")
+ #check burning speed
+ if self.speed:
+- self.cdrecord_line_append('speed=' + self.speed + ' ')
++ self.cdrecord_args.append("speed=%(speed)s" % vars(self))
+ else:
+ print _('no burning speed defined, using 2x')
+- self.cdrecord_line_append('speed=2' + ' ')
++ self.cdrecord_args.append("speed=2")
+ #check burn device
+ if self.device:
+- self.cdrecord_line_append('dev=' + self.device + ' ')
++ self.cdrecord_args.append("dev=%(device)s" % vars(self))
+ else:
+ print _('no device specified.')
+ sys.exit()
+ #for the ones who have buffer underrun protection
+ if self.burnfree:
+- self.cdrecord_line_append('driveropts=burnfree ')
++ self.cdrecord_args.append("driveropts=burnfree")
+ #enable multisession
+ if options.multisession:
+- self.cdrecord_line_append('-multi ')
++ self.cdrecord_args.append("-multi")
+ if options.data_cd or options.iso_cd:
+- self.cdrecord_line_append('-data ' + iso.dest)
++ self.cdrecord_args.extend(["-data", iso.dest])
+
+ def create(self):
+- """exec command in cdrecord_line"""
++ """exec command in cdrecord_args"""
+ print
+ print _('Press a key to begin recording...')
+ getch()
+ print _('Please wait...')
+- #print self.cdrecord_line
+- system(self.cdrecord_line)
++ subprocess.call(self.cdrecord_args)
+
+- def double_dao_create(self, cdrdao_line):
+- """exec command in cdrdao_line if you have TWO unit (reader and writer)"""
++ def double_dao_create(self, cdrdao_args):
++ """exec command in cdrdao_args if you have TWO unit (reader and writer)"""
+ print
+ print _('Place the source CD in the CD drive')
+ print _('and place a blank media in the recording unit.')
+ print _('Press a key to begin on-the-fly copy')
+ getch()
+- system(cdrdao_line)
+- #print cdrdao_line
++ subprocess.call(cdrdao_args)
+
+- def single_dao_create(self, cdrdao_line): #exec command in cdrdao_line
+- """exec command in cdrdao_line if you have only ONE unit (writer)"""
++ def single_dao_create(self, cdrdao_args):
++ """exec command in cdrdao_args if you have only ONE unit (writer)"""
+ print
+ print _('Place source CD in the recording unit.')
+ print _('Press a key to begin reading...')
+ getch()
+- system(cdrdao_line)
+- #print cdrdao_line
++ subprocess.call(cdrdao_args)
+
+ def another_copy(self):
+ """burn image untill user says no"""
+@@ -772,7 +774,7 @@
+ path = ''
+ path_preserved = ''
+ path_changed = ''
+- path_excluded = ''
++ paths_excluded = []
+ msinfo_values = ''
+ size = 0
+ first_time_multisession = 0
+@@ -834,12 +836,12 @@
+ if exists(join(root,i)):
+ if isfile(join(root,i)):
+ size = size - getsize(join(root,i))
+- path_excluded = path_excluded + '-x ' + '\'' + join(root,i) + '\'' + ' '
++ paths_excluded.append(join(root, i))
+ for d in dirs:
+ if fnmatch.fnmatch(d, x):
+ if exists(join(root,d)):
+ size = size - du(join(root,d), 0)
+- path_excluded = path_excluded + '-x ' + '\'' + join(root,d) + '\'' + ' '
++ paths_excluded.append(join(root, d))
+ print
+ print _('Size without exclusions: '), "\t", testsize/1048576, "Mb"
+
+@@ -851,18 +853,18 @@
+ sys.exit()
+ cdrom.size_compare(size, size)
+ #mkisofs line generation
+- iso.mkisofs_line_append(mkisofs + ' -R ') #-quiet
++ iso.mkisofs_args.extend([mkisofs, "-R"])
+ if exists(iso.tempdir):
+- iso.mkisofs_line_append('-o ' + iso.dest + ' ')
++ iso.mkisofs_args.extend(["-o", iso.dest])
+ else:
+ err(_('Error: ') + iso.tempdir + _(' does not exist'))
+ sys.exit()
+ #check Joliet option to be added
+ if iso.windows == 'yes':
+- iso.mkisofs_line_append('-J -joliet-long ')
++ iso.mkisofs_args.extend(["-J", "-joliet-long"])
+ #check exclude files - directories
+- if path_excluded:
+- iso.mkisofs_line_append(path_excluded + ' ')
++ for path_excluded in paths_excluded:
++ iso.mkisofs_args.extend(["-x", path_excluded])
+ if options.multisession:
+ multisession_choose = iso.ask_multisession()
+ if multisession_choose == 2:
+@@ -870,18 +872,21 @@
+ print _('Place target CD in CD/DVD writer unit and press a key...')
+ getch()
+ print _('Please wait...')
+- msinfo = commands.getstatusoutput(cdrecord + ' -msinfo dev=' + cdrom.device + ' 2>/dev/null')
+- iso.mkisofs_line_append(' -C ' + msinfo[1] + ' -M ' + cdrom.device + ' ')
++ command_args = [
++ cdrecord, "-msinfo",
++ "-dev=%(device)s" % vars(cdrom)]
++ command_process = subprocess.Popen(
++ command_args,
++ stdout=subprocess.PIPE, stderr=open(os.devnull))
++ msinfo = command_process.communicate()[0]
++ iso.mkisofs_args.extend(
++ ["-C", msinfo, "-M", cdrom.device])
+ elif not multisession_choose == 1:
+ sys.exit()
+ else:
+ first_time_multisession = 1
+- iso.mkisofs_line_append('-graft-points ' + global_path)
++ iso.mkisofs_args.extend(["-graft-points", global_path])
+ cdrom.line_create()
+- #a = cdrom.cdrecord_line_view()
+- #print a
+- #b = iso.mkisofs_line_view()
+- #print b
+ if exists(iso.dest):
+ if not iso.first_ask_remove():
+ cdrom.another_copy()
+@@ -925,37 +930,39 @@
+ if cdrom.device == cdrom.source_device or cdrom.source_device == '':
+ single_drive_mode = 1
+ #print single_drive_mode
+- cdrdao_line = cdrdao + ' '
++ cdrdao_args = [cdrdao]
+
+- #if options.simulate:
+- # cdrdao_line = cdrdao_line + "simulate "
+ if single_drive_mode == 1:
+- cdrdao_line = cdrdao_line + "copy "
++ cdrdao_args.append("copy")
+ if options.simulate:
+- cdrdao_line = cdrdao_line + "--simulate "
++ cdrdao_args.append("--simulate")
+ if options.eject:
+- cdrdao_line = cdrdao_line + "--eject "
+- cdrdao_line = cdrdao_line + "--datafile " + iso.dest + " --device " + cdrom.device + " "
++ cdrdao_args.append("--eject")
++ cdrdao_args.extend([
++ "--datafile", iso.dest,
++ "--device", cdrom.device])
+ if not cdrom.driver == '':
+- cdrdao_line = cdrdao_line + " --driver " + cdrom.driver + " "
+- cdrdao_line = cdrdao_line + " --speed " + cdrom.speed + " --fast-toc"
+- #print cdrdao_line
+- cdrom.single_dao_create(cdrdao_line)
++ cdrdao_args.extend(["--driver", cdrom.driver])
++ cdrdao_args.extend(
++ ["--speed", cdrom.speed, "--fast-toc"])
++ cdrom.single_dao_create(cdrdao_args)
+ else:
+- cdrdao_line = cdrdao_line + "copy "
++ cdrdao_args.append("copy")
+ if options.simulate:
+- cdrdao_line = cdrdao_line + "--simulate "
++ cdrdao_args.append("--simulate")
+ if options.eject:
+- cdrdao_line = cdrdao_line + "--eject "
+- cdrdao_line = cdrdao_line + "--device " + cdrom.device + " "
++ cdrdao_args.append("--eject")
++ cdrdao_args.extend(["--device", cdrom.device])
+ if not cdrom.driver == '':
+- cdrdao_line = cdrdao_line + " --driver " + cdrom.driver + " "
+- cdrdao_line = cdrdao_line + "--source-device " + cdrom.source_device + " "
++ cdrdao_args.extend(["--driver", cdrom.driver])
++ cdrdao_args.extend(["--source-device", cdrom.source_device])
+ if not cdrom.source_driver == '':
+- cdrdao_line = cdrdao_line + "--source-driver " + cdrom.source_driver + " "
+- cdrdao_line = cdrdao_line + " --speed " + cdrom.speed + " --on-the-fly --fast-toc "
+- #print cdrdao_line
+- cdrom.double_dao_create(cdrdao_line)
++ cdrdao_args.extend(
++ ["--source-driver", cdrom.source_driver])
++ cdrdao_args.extend([
++ "--speed", cdrom.speed,
++ "--on-the-fly", "--fast-toc"])
++ cdrom.double_dao_create(cdrdao_args)
+ sys.exit()
+ #------------------------------------------------------
+ if mode == 4: #AUDIO
+@@ -1083,8 +1090,11 @@
+ dev=ao.AudioDevice('wav', filename=normpath(iso.tempdir + 'burn_' + `counter`) +'.wav', overwrite=True)
+ if ext == '.mp3':
+ if external_decod > 0:
+- mp3_line = mp3_decoder + ' ' + mp3_decoder_option + ' ' + normpath(iso.tempdir + 'burn_' + `counter`) + '.wav ' + "\"" + y + "\""
+- system(mp3_line)
++ command_args = [mp3_decoder]
++ command_args.extend(shlex.split(mp3_decoder_option))
++ command_args.append(normpath(iso.tempdir + 'burn_' + repr(counter) + '.wav'))
++ command_args.append(y)
++ subprocess.call(command_args)
+ else:
+ pbar=progressBar(0,100,30)
+ af = mad.MadFile(y)
+@@ -1104,8 +1114,11 @@
+ elif ext == '.ogg':
+ SIZE = 4096
+ if external_decod > 0:
+- ogg_line = ogg_decoder + ' ' + ogg_decoder_option + ' ' + normpath(iso.tempdir + 'burn_' + `counter`) + '.wav ' + "\"" + y + "\""
+- system(ogg_line)
++ command_args = [ogg_decoder]
++ command_args.extend(shlex.split(ogg_decoder_option))
++ command_args.append(normpath(iso.tempdir + 'burn_' + repr(counter) + '.wav'))
++ command_args.append(y)
++ subprocess.call(command_args)
+ else:
+ pbar=progressBar(0,100,30)
+ af = ogg.vorbis.VorbisFile(y)
+@@ -1135,16 +1148,12 @@
+
+ #----------------- cdrecord line generation -------------------------
+
+- #building cdrecord audio line
+- for x in audio_list:
+- tracks = tracks + '\"' + x + '\"' + ' '
+- #tracks = tracks + x + ' '
+ #creates cdrecord line
+ cdrom.line_create()
+ #appendig tracks to cdrecord line
+- cdrom.cdrecord_line_append('-audio ' + tracks)
+- a = cdrom.cdrecord_line_view()
+- #print a
++ cdrom.cdrecord_args.append("-audio")
++ cdrom.cdrecord_args.extend(audio_list)
++
+ #burning CD
+ cdrom.create()
+ while ask_ok(_('Do you want to use processed audio files to create another Audio CD now? (y/N) '), False):
+
+=== modified file 'burn-configure'
+--- old/burn-configure 2005-03-14 21:38:46 +0000
++++ new/burn-configure 2009-08-23 04:03:23 +0000
+@@ -47,8 +47,8 @@
+
+ from optparse import OptionParser, OptionGroup
+ from os.path import *
+-from os import system, remove
+-import sys, ConfigParser, commands
++from os import remove
++import sys, ConfigParser
+ import os
+ import gettext
+ import pwd
+
diff -Nru burn-0.4.3/debian/patches/series burn-0.4.3/debian/patches/series
--- burn-0.4.3/debian/patches/series 1970-01-01 10:00:00.000000000 +1000
+++ burn-0.4.3/debian/patches/series 2009-08-23 15:44:01.000000000 +1000
@@ -0,0 +1 @@
+01.subprocess.patch
diff -Nru burn-0.4.3/debian/rules burn-0.4.3/debian/rules
--- burn-0.4.3/debian/rules 2009-08-23 15:12:43.000000000 +1000
+++ burn-0.4.3/debian/rules 2009-08-23 15:44:01.000000000 +1000
@@ -3,8 +3,10 @@
#export DH_VERBOSE=1
export DH_COMPAT=3
+include /usr/share/quilt/quilt.make
+
build: build-stamp
-build-stamp:
+build-stamp: ${QUILT_STAMPFN}
dh_testdir
# Add here commands to compile the package.
@@ -12,7 +14,7 @@
touch build-stamp
-clean:
+clean: unpatch
dh_testdir
dh_testroot
rm -f build-stamp configure-stamp
@@ -33,9 +35,8 @@
chmod 0755 `pwd`/debian/burn/usr/bin/*
cp -p burn.conf `pwd`/debian/burn/etc
cp -p burn.conf-dist `pwd`/debian/burn/usr/share/burn/
-
+
touch install-stamp
-
# Build architecture-independent files here.
binary-indep: build install
Attachment:
signature.asc
Description: Digital signature