Assigning to all packagers collectively, since there is no registered maintainer for this package.

CC: (none) => marja11
Assignee: bugsquad => pkg-bugs

Fixed on svn for cauldron by Shlomi Fish ( waiting for freeze push )

pushed on cauldron

Version: Cauldron => 5
Whiteboard: MGA5TOO => (none)

perltidy-20170521.0.0-1.mga5 	was submited to core 5 / updates_testing. Assigning to QA - please test.

Status: NEW => ASSIGNED
Assignee: shlomif => qa-bugs

Quoting from the link above (comment 0):
> Perl::Tidy tries to delete existing perltidy.ERR; but if deleting fails, it
> proceeds as if nothing happened. This can be abused to overwrite arbitrary
> files via symlink attack:
In the absence of the advisory we should assume that this is what has been fixed by the patch?

If so then the rest of the discussion on that link can be used as a basis for testing its success or otherwise, probably.  Back to this later.

CC: (none) => tarazed25

Advisory:
========================

Updated perltidy package fixes security vulnerability:

perltidy relies on the current working directory for certain output files and
does not have a symlink-attack protection mechanism, which allows local users
to overwrite arbitrary files by creating a symlink (CVE-2016-10374).

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-10374
https://lists.fedoraproject.org/archives/list/package-announce@lists.fedoraproject.org/thread/KS6UVEQTFR4NWHO6BVXCHZB2TFEJX7P3/
========================

Updated packages in core/updates_testing:
========================
perltidy-20170521.0.0-1.mga5

from perltidy-20170521.0.0-1.mga5.src.rpm

x86_64 in Mate, mga5, real hardware.

First look at the suggested test, following the text at the link already quoted, more or less verbatim.

$ sudo urpmi perl-Perl-Critic
$ mkdir tmp-pc
$ cd tmp-pc
$ mkdir tmp

Created file tmp/foo.pl
------------------------
$foo = 'bar'
$bar = 'baz';
------------------------

$ perlcritic -1 .
perltidy had errors!! at line 1, column 1.  See page 33 of PBP.  (Severity: 1)
Module does not end with "1;" at line 1, column 1.  Must end with a recognizable true value.  (Severity: 4)
Code not contained in explicit package at line 1, column 1.  Violates encapsulation.  (Severity: 4)
No package-scoped "$VERSION" variable found at line 1, column 1.  See page 404 of PBP.  (Severity: 2)
Code before strictures are enabled at line 1, column 1.  See page 429 of PBP.  (Severity: 5)
Code before warnings are enabled at line 1, column 1.  See page 431 of PBP.  (Severity: 4)

Updated perltidy.

$ echo foo > perltidy.ERR
$ perlcritic -1 .
# Same output as before, then:
$ cat perltidy.ERR
Perltidy version is 20170521

2: $bar = 'baz';
   ^            
found Scalar where operator expected

Missing ';' above?

May have to revisit this.

Functionality test using examples at https://www.tjhsst.edu/~dhyatt/superap/perl/ex5.html

Used sub3 and removed the indentations -> untidyfile

$ perltidy -o tidyfile untidyfile
$ cat tidyfile
#!/bin/perl
# SUB3:   Demonstrates how to return a scalar from a subroutine call.  The
#         module modifies a subrange of @arr1, and returns number of changes.

sub sub3 {
    my ($diff);
    print "  In sub3: Parms = @_ \n";    # Print out parameter string
    print "  In sub3:  ";
    for ( $_[0] .. $_[1] )               # Step through subrange, one by one
    {
        print $_, ". ", $arr1[$_], "   ";  # Default variable is current counter

        $arr1[$_] = $arr1[$_] . "XX";      # Concatenate operator is "." (dot)
    }

    print "\n";
    print "  In sub3: @arr1 \n";

    $diff = $_[1] - $_[0] + 1;             # Determine number of items used

    return $diff;                          # Assign return value to subroutine
}

Created attachment 9463 [details]
Example perl script with indentations removed.

$ perltidy -o tidyfile untidyfile
puts the indentations back.

Looks like I missed the point of the discussion by not intializing perltidy.ERR before the pre-update test.  Shall revisit this on another system.

Repeated the before-update test, ensuring that perltidy.ERR existed in the current directory, random content.
$ perlcritic -1 .
This analyses any perl file found in the current directory and below and reports a syntax error and records it in perltidy.ERR in the current directory, which is undesirable behaviour.

Repeating this after the update and using a dummy perltidy.ERR file the diagnosis is again written to the current directory.  Looks like the bug is not fixed.

If perltidy.ERR is deleted beforehand the result is the same.

Checked this in a Cauldron virtualbox and the results were the same.

Perltidy version is 20170521

(In reply to Len Lawrence from comment #10)
> Repeated the before-update test, ensuring that perltidy.ERR existed in the
> current directory, random content.
> $ perlcritic -1 .
> This analyses any perl file found in the current directory and below and
> reports a syntax error and records it in perltidy.ERR in the current
> directory, which is undesirable behaviour.
> 
> Repeating this after the update and using a dummy perltidy.ERR file the
> diagnosis is again written to the current directory.  Looks like the bug is
> not fixed.
> 
> If perltidy.ERR is deleted beforehand the result is the same.

was perltidy.ERR a symlink to a different file beforehand?

No, I was missing the plot again.  The perltidy.ERR file needs to be replaced by a symbolic link which the user cannot remove.  Have not found a way to do that.  

Linked perltidy.ERR to a copy of a file in another user's directory.  The link is replaced by the local diagnosis, thus breaking the link.

MGA5-32 on Asus A6000VM Xfce
No installation issues.
Changed tests above to see what the difference is between perlcritic and perltidy with regard to the .ERR file.
I put the foo.pl file and the untidy file in an otherwise empty folder.
Run "perlcritic -1 ." thus against the two files, generates output to the CLI, none to perltidy.ERR. Even if this .ERR file existed before this command (cat foo to it in the same folder as the test files), it deletes it.
Run "perlcritic -1 foo.pl" gives error feedback at the CLI and fills in the perltidy.ERR file,creating it if needed, overwriting it if present.
Run "perltidy -o tidyfile untidyfile" creates the tidyfile as indicated above, and does nothing to the perltidy.ERR file. Leaving it as is, if it existed before, not creating it if it was not there.
Now whether that is intended behavior, is beyond me.

CC: (none) => herman.viaene

Still trying to make sense of this.  I cannot remember where that reference to an undeleteable symlink came from.  What is certain is that the updated perlcritic is still overwriting a file in the current directory, even one with readonly permissions.  perltidy behaves as Herman says in comment 14.

If the intention of the update is to write diagnostic output to ~/tmp, ./tmp or /tmp then it looks like it fails and the update is not OK.

We could do with some clarification on this.

Shlomi, is there any guidance you can give here?

(In reply to David Walser from comment #16)
> Shlomi, is there any guidance you can give here?

Perhaps all we can do is file a bug upstream. Perhaps provide a reproducible testcase as a shell and/or perl program.

Thanks Shlomi.  Over to you Len.

Whiteboard: feedback => (none)

This still bemuses me.  No idea how to reproduce the symlink attack which was addressed by Debian I think.  See this extract from the perltidy change log:

Fixed debian #862667: failure to check for perltidy.ERR deletion can lead 
    to overwriting abritrary files by symlink attack. Perltidy was continuing 
    to write files after an unlink failure.  Thanks to Don Armstrong 
    for a patch.

That would be the fix that I don't know how to test.

RedHat Bug 1452050 - CVE-2016-10374
Status: CLOSED WONTFIX

This seems to indicate that the issue of writing the error file to the current directory is not important and from the above not actually relevant to this bug.

So, as in many other cases we have to assume that the patch is effective and give an OK on the strength of that.  Holding off in case somebody objects.

@Herman - comment 14.
Are you happy to add the 32-bit OK?

Doing so.

Whiteboard: MGA5-64-OK => MGA5-64-OK MGA5-32-OK

An update for this issue has been pushed to the Mageia Updates repository.

http://advisories.mageia.org/MGASA-2017-0301.html

Status: ASSIGNED => RESOLVED
Resolution: (none) => FIXED