Bug 7806 - Tainting through concatenation with $^X does not taint
Summary: Tainting through concatenation with $^X does not taint
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: spamassassin (show other bugs)
Version: 3.4.4
Hardware: PC Linux
: P2 blocker
Target Milestone: Undefined
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-04-08 14:38 UTC by Rodolfo Saccani
Modified: 2020-04-09 12:41 UTC (History)
2 users (show)



Attachment Type Modified Status Actions Submitter/CLA Status
sample code to reproduce the issue application/x-perl None Rodolfo Saccani [NoCLA]

Note You need to log in before you can comment on or make changes to this bug.
Description Rodolfo Saccani 2020-04-08 14:38:47 UTC
Env: CentOS Linux, perl5.10.1, spamassassin invoked by MailScanner

Util.pm, sub taint_var is supposed to taint a variable by concatenating it with $^X:
###########################################################################
sub taint_var {
  my ($v) = @_;
  return $v unless defined $v;      # can't taint "undef"

  # $^X is apparently "always tainted".
  # Concatenating an empty tainted string taints the result.
  return $v . substr($^X, 0, 0);
}

But it doesn't. Variables are not tainted by concatenation with $^X

The following implementation does indeed taint:
###########################################################################
my $tainted =  undef;
sub taint_var {
  my ($v) = @_;
  return $v unless defined $v;      # can't taint "undef"

  # Create a handy tainted empty string
  unless (defined $tainted) {
    open my $fh, '<', \"" or die "Can't open: $!";
    local $/;   
    $tainted= <$fh>;
  }

  # Concatenating an empty tainted string taints the result.
  return $v . substr($tainted, 0, 0);
}

Rather than using $^X this approach creates a certainly tainted variable $tainted only once and re-uses it whenever needed.
Comment 1 Henrik Krohns 2020-04-08 19:42:19 UTC
(In reply to Rodolfo Saccani from comment #0)
> 
> But it doesn't. Variables are not tainted by concatenation with $^X

And where is your proof please?
Comment 2 Rodolfo Saccani 2020-04-08 20:35:36 UTC
MailScanner --lint  
this is all is needed to reproduce the issue on CentOS with perl 5.10.1
Comment 3 Henrik Krohns 2020-04-08 20:59:44 UTC
(In reply to Rodolfo Saccani from comment #2)
> MailScanner --lint  
> this is all is needed to reproduce the issue on CentOS with perl 5.10.1

Sorry but this is not helpful. I do not have MailScanner and I don't know what is supposed to happen.

What is the actual _error output_?

Fact is that CentOS6 box stock perl 5.10.1 works just fine with $^X. Your problems is likely something other MailScanner related.

$ perl -T -e '$ENV{PATH} = "/usr/bin"; $foo = "uptime"; system($foo);'
 22:53:52 up 22 days, 10:59,  1 user,  load average: 0.02, 0.01, 0.00
$ perl -T -e '$ENV{PATH} = "/usr/bin"; $foo = "uptime".substr($^X, 0, 0); system($foo);'
Insecure dependency in system while running with -T switch at -e line 1.
Comment 4 Rodolfo Saccani 2020-04-09 08:41:19 UTC
Created attachment 5696 [details]
sample code to reproduce the issue

This attachment reproduces the issue.
Launch it as root.

# perl taint.pl 
Setting UID to 89
Use taint?.............1
Is $^X tainted?........0
Is $tainted tainted?...1

Why?
When dropping root privileges the taint checks are enabled but $^X is not tainted because it had been executed previously.

This is expected, read below.
https://perldoc.perl.org/perlsec.html#Taint-mode) says:
Perl automatically enables a set of special security checks, called taint mode, when it detects its program running with differing real and effective user or group IDs.

This leads to $^X not being reliable when taint is enabled at runtime.
Enabling taint checking at runtime is not unusual.

I suggest to replace use of $^X with the code provided, which taints reliably.
Comment 5 Henrik Krohns 2020-04-09 12:41:37 UTC
Doesn't seem to be a problem with newer perls..

Oh well, I rather use both methods then, never know what Perl might change:

Sending        spamassassin-3.4/lib/Mail/SpamAssassin/Util.pm
Sending        trunk/lib/Mail/SpamAssassin/Util.pm
Transmitting file data ..done
Committing transaction...
Committed revision 1876320.