processmail doesn't work with the P4DTI

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Email Notifications
P2
normal
RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: Gareth Rees, Assigned: bbaetz)

Tracking

2.14
Bugzilla 2.16

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
BACKGROUND

The Perforce Defect Tracking Integration <http://www.ravenbrook.com/project/
p4dti/> integrates Bugzilla with the Perforce software configuration management 
system.  It replicates bugs between Bugzilla and Perforce so that users of 
Perforce can see and edit their bugs, and relate bugs to source changes.

When someone changes a bug in Perforce, the P4DTI updates the bug in Bugzilla.  
It then needs to send e-mail notifications, which it does by running the Bugzilla 
processmail script.

DEFECT DESCRIPTION

When the P4DTI runs the processmail script from Bugzilla 2.14, no e-mail is sent.  
Instead, the following error message appears in the standard error:

Insecure $ENV{BASH_ENV} while running with -T switch at ./processmail line 745 \
(#1)

    (F) You can't use system(), exec(), or a piped open in a setuid or
    setgid script if any of $ENV{PATH}, $ENV{IFS}, $ENV{CDPATH},
    $ENV{ENV} or $ENV{BASH_ENV} are derived from data supplied (or
    potentially supplied) by the user.  The script must set the path to a
    known value, using trustworthy data.  See perlsec.

REPRODUCING THIS BUG

You probably don't want to install the P4DTI in order to confirm this bug.  
Instead, run Bugzilla in an environment where some of the dangerous environment 
variable have values.  You'll see that processmail doesn't send e-mail.

DIAGNOSIS

The processmail script is invoking the shell in order to open a pipe to sendmail.  
It does so with taint mode turned on (-T option to perl) but without clearing the 
dangerous environment variables.

SOLUTION

Clear the dangerous environment variables, as recommended in the perlsec manpage, 
by running

      delete @ENV{qw(IFS CDPATH ENV BASH_ENV)};

before invoking the shell.

I added this line to my processmail script and the P4DTI was then able to send e-
mail notifications.

OTHER NOTES

Why doesn't this bug show up in normal operation?  Perhaps because Bugzilla 
always runs in an environment where the dangerous environment variables aren't 
set (e.g., in Apache under user "nobody").

This is related to bug 59349.

Comment 1

17 years ago
Yes, in bug 53949 delete $ENV{path}; was added to globals.pl, but the other
environment variables were not.  These variables apparently do not exist when
apache runs the processmail script (as you pointed out) so they don't cause
bailing under "normal" circumstances.  I see no problem with adding these extra
deletions where PATH is currently deleted.
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
(Assignee)

Comment 2

16 years ago
I fixed this as part of bug 97469 - I got sick of having to unset BASH_ENV to
test processmail from the cmd line
Assignee: jake → bbaetz
Depends on: 97469
(Assignee)

Comment 3

16 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

16 years ago
oops - reopening. ddk pointed out that you wanted other env vars removed, too
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 5

16 years ago
Yes, BASH_ENV was the one that I observed, but it seems wise to clear them all to prevent problems in future.
(Assignee)

Comment 6

16 years ago
Created attachment 55509 [details] [diff] [review]
patch
Questions:

How did you make sure this list was complete?
Why don't you remove PATH on the same line as everything else?
(Assignee)

Comment 8

16 years ago
> How did you make sure this list was complete?

See the error message.

> Why don't you remove PATH on the same line as everything else?

No particular reason, except that PATH can be modified, while the others are
just unwanted.
Status: REOPENED → ASSIGNED
Why use two lines where one will do? Add PATH to the list. :-)

By the way, gdr@ravenbrook.com - that's the best bug report I've seen in ages :-)

Gerv
(Assignee)

Comment 10

16 years ago
Created attachment 57445 [details] [diff] [review]
new patch
Attachment #55509 - Attachment is obsolete: true
Comment on attachment 57445 [details] [diff] [review]
new patch

r=gerv.

Gerv
Attachment #57445 - Flags: review+
Attachment #57445 - Flags: review+
(Assignee)

Comment 12

16 years ago
Checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

16 years ago
Created attachment 58204 [details] [diff] [review]
bah

perl 5.005 appears to have a bug, and warns on this:

Use of implicit split to @_ is deprecated 
(D deprecated) It makes a lot of work for the compiler when you clobber a
subroutine's argument list, so it's better if you assign the results of a
split() explicitly to an array (or list).

Bad internal code? Anyway, this fixes it.
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.