Bug 1378885

Upstream 0.9908 version does not suffer from this problem.

Thanks Petr.

Any plans to update to 0.9908 any time soon in RHEL7? RHEL7u3?

Could you please provide me with a testing package so that I can confirm?

I'm sorry for a positive message. It's not corrected in any upstream version. There are two version module implementations (a pure-Perl vpp and XS vxs) and it looks like the XS one suffers from this issue. But not when running on recent perl.

A quick test appeared to confirm that 9908 didn't show the issue reproduced by the test script on RHEL7, all else being constant:

<snip>
[root@dell-r430-6 SPECS]# rpm -q perl-version
perl-version-0.99.07-2.el7.x86_64
[root@dell-r430-6 SPECS]# ./test.pl 
Invalid version format (non-numeric data) at ./test.pl line 6.
[root@dell-r430-6 SPECS]# rpm -Uvh ~/perl-version-0.99.08-1test.el7_2.x86_64.rpm 
Preparing...                          ################################# [100%]
Updating / installing...
   1:perl-version-3:0.99.08-1test.el7_################################# [ 50%]
Cleaning up / removing...
   2:perl-version-3:0.99.07-2.el7     ################################# [100%]
[root@dell-r430-6 SPECS]# rpm -q perl-version
perl-version-0.99.08-1test.el7_2.x86_64
[root@dell-r430-6 SPECS]# ./test.pl 
[root@dell-r430-6 SPECS]# 
</snip>

Is this expected since (I don't believe) we're using the vxs interface here?

We should use vxs. Strace the old version to see no vpp module is loaded. I don't undetstand yet why the the fallback happens.

Moreover the reproducer in the original report is incomplete. It's necessary to check the string value of version->new($a) is equaled to "1.2.3". This is not true for 0.9908 that returns empty string.

I located the change in the perl. Large commit:

4bac9ae47b5ad7845a24e26b0e95609805de688a is the first bad commit
commit 4bac9ae47b5ad7845a24e26b0e95609805de688a
Author: Chip Salzenberg <chip>
Date:   Fri Jun 22 15:18:18 2012 -0700

    Magic flags harmonization.
[...]
    This patch removes the shift mechanic for magic flags, thus exposing (and
    fixing) some improper usage of magic SVs in which mg_get() was not called
    correctly.  It also has the side effect of making magic get functions
    specifically set their SVs to undef if that is desired, as the new behavior
    of empty get functions is to leave the value unchanged.  This is a feature,
    as now get magic that does not modify its value, e.g. tainting, does not
    have to be special cased.


changes various internals and this is not definitely something we want to port into perl. It causes vxs version implementation to handle tainted values.

I think the version module was never intended to handle tainted values. I will ask upstream what he thinks about it.

(In reply to Petr Pisar from comment #6)
> I don't undetstand yet why the the fallback happens.
>
That's fall back happened because version::vxs did not match version module because code from upstream mercurial repository will often forget to update the $version::vxs::VERSION value.

The change in version responsible for not croaking about "non-numeric data" is this commit:

changeset:   480:efb44fbc6bd8
user:        John Peacock <john.peacock>
date:        Sat Feb 01 13:30:16 2014 -0500
summary:     Deal with certain tiedscalars (e.g. created by Readonly::XS).

It added this code:

--- a/vutil/vutil.c     Sat Feb 01 08:55:41 2014 -0500
+++ b/vutil/vutil.c     Sat Feb 01 13:30:16 2014 -0500
[...]
@@ -605,7 +609,11 @@
        version = savesvpv(ver);
        SAVEFREEPV(version);
     }
-    else if ( SvPOK(ver) )/* must be a string or something like a string */
+    else if ( SvPOK(ver)
+#if PERL_VERSION_LT(5,17,2)
+             || (SvTYPE(ver) == SVt_PVMG && SvPOKp(ver))
+#endif
+           )/* must be a string or something like a string */
     {
        STRLEN len;
        version = savepvn(SvPV(ver,len), SvCUR(ver));

That's responsible crunching the tainted string, thus not croaking. But still because of missing the above-mentioned perl change it does not retrieve the value and stores an empty string instead.

By the way, how exactly is this bug a regression?

As far as I know, the first RHEL-7 prerelease had perl-version-0.99.02-3.el7 that does not die with the error, but returns an empty string and that's also incorrect behavior. Then first official RHEL-7 release delivered perl-version-0.99.07-1.el7 that dies with the reported error and it does not differ from current perl-version-0.99.07-2.el7.x86_64.

It probably behaves differently from RHEL-6, but Red Hat does not guarantee Perl compatibility across multiple RHEL major versions.

I talked to version module author, and he tends to ban the tainted input completely. That means the current perl-version-0.99.07-2.el7.x86_64 behavior plus ban in the pure-perl variant could become the new official behavior. If you have different opinion how the future (= upstream) version should behave, you can express your ideas in the upstream report <http://rt.cpan.org/Public/Bug/Display.html?id=118087>.

Thanks Petr.

In the meantime we will have a look whether to disable tainting for module in question which fails due to this regression.

... by regression I mean regression from ordinary Perl behaviour across RHELs. If it is broken since the first release of RHEL7 then it is still a regression/bug as this behaviour is obviously not a feature but broken functionality.

Just for the record - updating the reproducer to:

#!/usr/bin/perl -Tw
use version;
$ENV{PATH}='/usr/bin';
$a=`echo 1.2.3`;
chomp $a;
#($a) = $a =~ /(.*)/;
$v = version->new($a);
$vStr = $v->stringify();
if ($a eq $vStr) {
    print "Version is good\n"
} else {
    print "Version '$vStr' is bad\n";
    exit 1;
}

which is also sensitive to an empty version string (thanks to Mark Wilson).

Created attachment 1205220 [details]
Support for parsing tainted input (first part)

This is the upstream code from 0.9908 not to die on a tainted input.

Created attachment 1205222 [details]
Support for stringyfing tainted version (second part)

This is a proposed fix to allow printing version object that was created from tainted input. The output string will be also tainted as the input.

I developed a fix that adds support for parsing tainted input and stringifying such objects. An unsupported testing package is available on <http://people.redhat.com/~ppisar/perl-version-0.99.07-3.el7/>.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:0663