If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Insecure dependency error when trying to update aliases (problem with multi-select custom fields)

RESOLVED FIXED in Bugzilla 3.4

Status

()

Bugzilla
Creating/Changing Bugs
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: gerv, Assigned: Frédéric Buclin)

Tracking

Bugzilla 3.4
Bug Flags:
approval +
approval3.4 +
blocking3.4.3 +

Details

Attachments

(1 attachment)

949 bytes, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
URLs like this one:

http://localhost/src/bugzilla-3.4/process_bug.cgi?alias=newalias&token=1256233218-c8c6f563a8e00f57ac3917e5425175c1&id=51

(i.e. updating the alias on a bug)

produce errors like this:

Software error:

Insecure dependency in parameter 1 of DBI::db=HASH(0x9b35c80)->selectcol_arrayref method call while running with -T switch at Bugzilla/Bug.pm line 3688.

(That line number is for 3.4-BRANCH).

That line says:
          $self->{$attr} ||= Bugzilla->dbh->selectcol_arrayref(
              "SELECT value FROM bug_$attr WHERE bug_id = ? ORDER BY value",
              undef, $self->id);

The value of $attr at the time the error occurs is cf_multiple, one of my custom fields.

Inserting the line:
          trick_taint($attr);
just before fixes it, but I have no idea if that's just masking the problem. Nor do I know how this problem is connected with attempts to change aliases. But that's what triggers it. If I add "&defined_cf_multiple=" to the URL (yes, with no value, just the key), it goes away.

Gerv
(Assignee)

Comment 1

8 years ago
I can reproduce. I'll investigate.
Flags: blocking3.4.3+
Target Milestone: --- → Bugzilla 3.4
(Assignee)

Comment 2

8 years ago
That's crazy. In Bugzilla::Bug::AUTOLOAD, the following line taints $attr:

 return $self->{$attr} if defined $self->{$attr};

Right before this line, $attr is untainted (as it comes from the DB), but right after it, it's tainted?!

It's not a good thing to put trick_taint() here.

Comment 3

8 years ago
That doesn't make any sense. Hash keys are actually always *untainted*. That sounds like a bug in Perl itself, to me.
(Assignee)

Comment 4

8 years ago
Created attachment 407871 [details] [diff] [review]
patch, v1

I can only reproduce the taint issue with Perl 5.10.0 (which is also the Perl version used by Gerv). This problem doesn't occur with Perl 5.8.8 nor with 5.10.1. Per our discussion on IRC, $attr passed _validate_attribute(), so it's indeed safe to detaint it explicitly at this point. This hack can go away once we require Perl 5.10.1 or newer, i.e. in a very long time. :)
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #407871 - Flags: review?(mkanat)

Comment 5

8 years ago
Comment on attachment 407871 [details] [diff] [review]
patch, v1

Looks good to me! :-)
Attachment #407871 - Flags: review?(mkanat) → review+

Updated

8 years ago
Flags: approval3.4+
Flags: approval+
(Assignee)

Comment 6

8 years ago
tip:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.290; previous revision: 1.289
done

3.4.2:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.276.2.11; previous revision: 1.276.2.10
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.