Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Cyril Brulebois]

Control: tag -1 pending

gregor herrmann  (2017-06-30):
> Thank you; uploaded (before going to bed and without sending _this_
> mail :))

Now flagged for acceptance, thanks.


KiBi.


signature.asc
Description: Digital signature

Processed: Re: Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Debian Bug Tracking System]

Processing control commands:

> tag -1 pending
Bug #863049 [release.debian.org] jessie-pu: package shutter/0.92-0.1+deb8u2
Added tag(s) pending.

-- 
863049: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863049
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[gregor herrmann]

On Fri, 30 Jun 2017 02:03:34 +0200, Cyril Brulebois wrote:

> > So I propose to proceed with the upload to jessie with the proposed
> > changes, if that's ok for KiBi.
> Sure, feel free to go ahead.

Thank you; uploaded (before going to bed and without sending _this_
mail :))

Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Ry Cooder: Available Space


signature.asc
Description: Digital Signature

Processed: Re: Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Debian Bug Tracking System]

Processing control commands:

> tag -1 - moreinfo + confirmed
Bug #863049 [release.debian.org] jessie-pu: package shutter/0.92-0.1+deb8u2
Removed tag(s) moreinfo.
Bug #863049 [release.debian.org] jessie-pu: package shutter/0.92-0.1+deb8u2
Added tag(s) confirmed.

-- 
863049: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863049
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Cyril Brulebois]

Control: tag -1 - moreinfo + confirmed

gregor herrmann  (2017-06-29):
> On Thu, 29 Jun 2017 09:11:43 +0200, Dominique Dumont wrote:
> 
> > On Wednesday, 28 June 2017 20:15:01 CEST gregor herrmann wrote:
> > > I suppose yes, in order to make sure that the script waits for
> > > nautilus-sendto to return, as the return value is checked in the next 
> > > line.
> > Indeed. The only drawback is that shutter will hang while the mail is sent 
> > by 
> > nautilus-sendto. Depending on network condition, this may be noticeable by 
> > user.
> 
> Thanks for the confirmation.
>  
> 
> So I propose to proceed with the upload to jessie with the proposed
> changes, if that's ok for KiBi.

Sure, feel free to go ahead.


KiBi.


signature.asc
Description: Digital signature

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[gregor herrmann]

On Thu, 29 Jun 2017 09:11:43 +0200, Dominique Dumont wrote:

> On Wednesday, 28 June 2017 20:15:01 CEST gregor herrmann wrote:
> > I suppose yes, in order to make sure that the script waits for
> > nautilus-sendto to return, as the return value is checked in the next line.
> Indeed. The only drawback is that shutter will hang while the mail is sent by 
> nautilus-sendto. Depending on network condition, this may be noticeable by 
> user.

Thanks for the confirmation.
 

So I propose to proceed with the upload to jessie with the proposed
changes, if that's ok for KiBi.


Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Bruce Springsteen & The E Street Band: Brilliant Disguise


signature.asc
Description: Digital Signature

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Dominique Dumont]

On Wednesday, 28 June 2017 20:15:01 CEST gregor herrmann wrote:
> I suppose yes, in order to make sure that the script waits for
> nautilus-sendto to return, as the return value is checked in the next line.

Indeed. The only drawback is that shutter will hang while the mail is sent by 
nautilus-sendto. Depending on network condition, this may be noticeable by 
user.

I guess that Gtk2 offers a way to fork process and check the result without 
hanging, but that would require more work (and ramp-up on Gtk2)

All the best

-- 
 https://github.com/dod38fr/   -o- http://search.cpan.org/~ddumont/
http://ddumont.wordpress.com/  -o-   irc: dod at irc.debian.org

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[gregor herrmann]

On Wed, 28 Jun 2017 01:27:42 +0200, Cyril Brulebois wrote:

> gregor herrmann  (2017-05-20):
> > I've prepared an upload of shutter for stable. The new version
> > includes two patches:
> > - one fixing CVE-2016-10081 / #849777
> > - another one which dod uploaded together with this one as 0.93.1-1.3
> >   in January which is also security relevant (replaces
> >   system("string") with system(@array)).
> That's a long patch… Comments below (see last hunk, mainly).

Thanks for taking the time to go through the patch in detail!
 
> > + sub nautilus_sendto {
> > +   my ( $self, $user_data ) = @_;
> > +-  system("nautilus-sendto $user_data &");
> > ++  system('nautilus-sendto', $user_data);
> > +   if($?){
> > +   my $response = $self->{_dialogs}->dlg_error_message( 
> > +   sprintf( $self->{_d}->get("Error while executing %s."), 
> > "'nautilus-sendto'"),
> 
> Was the '&' really meant to go away?

I suppose yes, in order to make sure that the script waits for nautilus-sendto
to return, as the return value is checked in the next line.

And/or because it simply doesn't work, as adding a '&' would be
interpreted as an argument:


#v+
#!/usr/bin/perl

use strict;
use warnings;

my $args='-ls';

print "string\n";
system( "ls $args &" ) == 0 or die "system(string) failed: $?";
#-

% perl background.pl
string
total 4 
  
4 -rw-rw-r-- 1 gregoa gregoa 234 Jun 28 20:10 background.pl


vs.


#v+
#!/usr/bin/perl

use strict;
use warnings;

my $args='-ls';

print "list\n";
system( 'ls', '-la', '&' ) == 0 or die "system(list) failed: $?";
#v-

% perl background.pl
list
ls: cannot access '&': No such file or directory
system(list) failed: 512 at background.pl line 9.


So yes, this seems intended :)


Nevertheless looping in dod as the author of this patch.


Cheers,
gregor


-- 
 .''`.  https://info.comodo.priv.at/ - Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member of VIBE!AT & SPI, fellow of the Free Software Foundation Europe
   `-   NP: Ben Weaver: Voice In The Wilderness


signature.asc
Description: Digital Signature

Processed: Re: Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Debian Bug Tracking System]

Processing control commands:

> tag -1 moreinfo
Bug #863049 [release.debian.org] jessie-pu: package shutter/0.92-0.1+deb8u2
Added tag(s) moreinfo.

-- 
863049: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=863049
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[Cyril Brulebois]

Control: tag -1 moreinfo

gregor herrmann  (2017-05-20):
> I've prepared an upload of shutter for stable. The new version
> includes two patches:
> - one fixing CVE-2016-10081 / #849777
> - another one which dod uploaded together with this one as 0.93.1-1.3
>   in January which is also security relevant (replaces
>   system("string") with system(@array)).

That's a long patch… Comments below (see last hunk, mainly).

> +shutter (0.92-0.1+deb8u2) UNRELEASED; urgency=medium

As usual, target jessie when uploading.

> ++system(
> ++convert =>
> ++-caption => $text,
> ++-fill => sprintf( "#%04x%04x%04x%04x",
> ++  $color->red,
> ++  $color->green,
> ++  $color->blue,
> ++  $stroke_color->get_alpha
> ++  ),
> ++$filename,
> ++-pointsize => $pointsize_sbutton->get_value,
> ++-gravity => $gravity_combo->get_active_text,
> ++qw/-bordercolor snow -background black/,
> ++-polaroid => $angle_sbutton->get_value,
> ++$tmpfilename
> ++);

Nice variations on the “how to build a list” topic, thanks for making
sure the release team folks are fluent in Perl.

> ++#execute imagemagick command
> ++system(
> ++convert =>
> ++-background => '#',

I wasn't sure that worked, but that seems to do the trick; Perl is fun.

> + sub nautilus_sendto {
> + my ( $self, $user_data ) = @_;
> +-system("nautilus-sendto $user_data &");
> ++system('nautilus-sendto', $user_data);
> + if($?){
> + my $response = $self->{_dialogs}->dlg_error_message( 
> + sprintf( $self->{_d}->get("Error while executing %s."), 
> "'nautilus-sendto'"),

Was the '&' really meant to go away?


KiBi.


signature.asc
Description: Digital signature

Bug#863049: jessie-pu: package shutter/0.92-0.1+deb8u2

[gregor herrmann]

Package: release.debian.org
Severity: normal
Tags: jessie
User: release.debian@packages.debian.org
Usertags: pu

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

I've prepared an upload of shutter for stable. The new version
includes two patches:
- - one fixing CVE-2016-10081 / #849777
- - another one which dod uploaded together with this one as 0.93.1-1.3
  in January which is also security relevant (replaces
  system("string") with system(@array)).

Full debdiff attached.


Cheers,
gregor

-BEGIN PGP SIGNATURE-

iQKTBAEBCgB9FiEE0eExbpOnYKgQTYX6uzpoAYZJqgYFAlkgqZJfFIAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEQx
RTEzMTZFOTNBNzYwQTgxMDREODVGQUJCM0E2ODAxODY0OUFBMDYACgkQuzpoAYZJ
qgbWNA/8DPTaKOesYQ4tMjj580foTMqUu+G3qEk/UgqFhkhAFNoBXRJKVPiF4tkL
V10CgEpEySMee18bVLSk08nWb0NJmuP8OSlcw43nspI2nRZIum7Vnsyf9rhOEd2o
0SzW8Z1/cNsRWhgy2UP5esinZmu9/djJBmSfc5E7bzH3tIYHc0H2wfnhys3uYOLk
yyoCZf5u1JA/cTFRcIgGmX60PhrtZMPRtN3x63JYKCOqGPR4rBrb9aii4etKetEh
lXMj8hF2ZShnJDXTXuI+rtq5i3KPuAhBr5bjqXuQJ4g2C8L1KHG9HYQB5XZNaafn
7oOa39fQXVA52hf/WBM6y+YICkO2EuVS+6bNcEJfWQaQN19NX/YdpkkzxCyuv3/Q
rMNRHt9B3tZDeD99tGhvG8RbKyZbfbML+xfcOELLXNZhf+LjWb6hE4rtLJUirgB6
7FmiQwvOLPZc1tHXjLGEC6cvUt7jI95ZOCiQc/OtxrWPmRmJH88mYQJP1EQA0+gm
RBQf6PlDvKMN+9/zPZxhK4lZbuQz6NN27B1De9f5kB2hESQphzCBpVqSs4ytsekc
hGYGFt98igVGrZo244Of3FOSCZd0fxFCzaxOm0R2op/Z+AQgMmhiPY+0bO2fWYTc
dkbLqtxwNZQEh3vesb08NN1bZUeXRshybi85g9TIam5QQBKFs64=
=KPfu
-END PGP SIGNATURE-
diff --git a/debian/changelog b/debian/changelog
index 009a696..af656ed 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+shutter (0.92-0.1+deb8u2) UNRELEASED; urgency=medium
+
+  [ Dominique Dumont ]
+  * add patch to fix CVE-2016-10081 (Closes: #849777)
+  * add patch to secure system() calls
+
+ -- gregor herrmann   Sat, 20 May 2017 22:30:53 +0200
+
 shutter (0.92-0.1+deb8u1) jessie; urgency=high
 
   * Fix insecure usage of system(). Closes: #798862 [CVE-2015-0854]
diff --git a/debian/patches/CVE-2016-10081.patch 
b/debian/patches/CVE-2016-10081.patch
new file mode 100644
index 000..edd2ff8
--- /dev/null
+++ b/debian/patches/CVE-2016-10081.patch
@@ -0,0 +1,42 @@
+Bug: https://bugs.launchpad.net/shutter/+bug/1652600
+Bug-Debian: https://bugs.debian.org/849777
+Author: Christoph Biedl 
+Description: fix insecure use of perl exec()
+ The patch attached uses the multi-argument invocation and also changes
+ it in the code path for non-Perl plugins.
+--- a/bin/shutter
 b/bin/shutter
+@@ -7159,8 +7159,13 @@
+ elsif ( $pid == 0 ) {
+ 
+ #see Bug #661424
+-my $qfilename = quotemeta $session_screens{$key}->{'long'};
+-exec( sprintf( "$^X $plugin_value %d $qfilename 
$session_screens{$key}->{'width'} $session_screens{$key}->{'height'} 
$session_screens{$key}->{'filetype'}\n", $socket->get_id ) );
++#my $qfilename = quotemeta $session_screens{$key}->{'long'};
++exec( $^X, $plugin_value,
++$socket->get_id,
++$session_screens{$key}->{'long'},
++$session_screens{$key}->{'width'},
++$session_screens{$key}->{'height'},
++$session_screens{$key}->{'filetype'} );
+ }
+ 
+ $sdialog->show_all;
+@@ -7193,11 +7198,15 @@
+ my $plugin_process = Proc::Simple->new;
+ 
+ #see Bug #661424
+-my $qfilename = quotemeta $session_screens{$key}->{'long'};
++#my $qfilename = quotemeta $session_screens{$key}->{'long'};
+ 
+ $plugin_process->start(
+ sub {
+-system("'$plugin_value' $qfilename 
'$session_screens{$key}->{'width'}' '$session_screens{$key}->{'height'}' 
'$session_screens{$key}->{'filetype'}' ");
++system( $plugin_value,
++$session_screens{$key}->{'long'},
++$session_screens{$key}->{'width'},
++$session_screens{$key}->{'height'},
++$session_screens{$key}->{'filetype'} );
+ POSIX::_exit(0);
+ }
+ );
diff --git a/debian/patches/fix-perl-system-calls 
b/debian/patches/fix-perl-system-calls
new file mode 100644
index 000..6de4ac5
--- /dev/null
+++ b/debian/patches/fix-perl-system-calls
@@ -0,0 +1,268 @@
+Author: dod
+Description: Fix perl system calls
+ This patch replaces all system("big string") calls to 
+ system(@big_list) in all plugins to avoid problems similar to CVE-2016-10081.
+--- a/share/shutter/resources/system/plugins/perl/sppolaroid/sppolaroid
 b/share/shutter/resources/system/plugins/perl/sppolaroid/sppolaroid
+@@ -349,9 +349,6 @@
+ 
+ sub apply_effect {
+ 
+-  #quote filename
+-  my $qfilename = quotemeta $filename;
+-
+   if ( $use_caption ) {
+ 
+   my $text = $caption_entry->get_text;
+@@ -360,27