Undefined subroutine &main::detaint_natural called at editusers.cgi line 739.

RESOLVED FIXED in Bugzilla 2.14

Status

()

Bugzilla
Administration
P1
blocker
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: Christopher Aillon (sabbatical, not receiving bugmail), Assigned: David D. Kilzer (ddk))

Tracking

2.14.1
Bugzilla 2.14

Details

(Whiteboard: 2.14.1 blocker)

Attachments

(1 attachment, 1 obsolete attachment)

While using the current 2.14.1 branch, remove or add a user to a group and you
receive that message.
IMO this should be a 2.14.1 release blocker if someone can confirm this.
Whiteboard: 2.14.1 blocker ?
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.14
sub detaint_natural was added in globals.pl in rev 1.124bbaetz%cs.mcgill.caOct
23 2001 in bug 97469 which the branch didn't pick up.
And the calls (two of them) in editusers.cgi were added in bug 108821: Prevent
users with any blessgroupset privileges from blessing any group set, which the
branch DID pick up.

1.29 myk%mozilla.org Nov 7 2001
long_list.cgi also has a call to detaint_natural.

Fix for bug 109690: long_list.cgi was not validating that the bug ID parameter
was actually a number.
Patch by Brad Baetz
r= justdave, gerv
Group: security?
Sigh. Yeah, port detaint_natural to the branch - these security bugs are why it 
exists. I'm surprised we didn't catch this sooner.

(BTW, why is this a security-sensitive bug? Of course, thats the only reason I 
noticed it while on holiday :)
Because the summaries of a couple of the existing security bugs were mentioned
in previous comments.

Comment 7

16 years ago
I am holding putting out a Red Hat Bugzilla Errata update pending fix for this
bug.
(Assignee)

Comment 8

16 years ago
Created attachment 63263 [details] [diff] [review]
Patch v.1

Picking up detaint_natural() and trick_taint() from patch for
bug 97469 (attachment 54138 [details] [diff] [review]).  Patches only globals.pl and
processmail.

globals.pl
  - Add attribution for Bradley Baetz
  - Pick up additional $ENV deletion
  - Replace sub detaint_string() with sub trick_taint() and
    sub detaint_natural()
  - Changes use of detaint_string() to trick_taint()

processmail
  - Add attribution for Bradley Baetz [not in original patch]
  - Change detaint_string() to trick_taint()
  - Add two calls to detaint_natural()
  - Add call to detaint_natural() in foreach loop

I recommend that you look at the original patch when reviewing
this one!

Comment 9

16 years ago
Is there anything in 2.14.1 that wants to use trick_taint()?  If not, I'd
suggest simply adding detaint_natural() and leaving my poorly named
detaint_string() alone for this version :)

2.16 will, obviously, have the correctly named routine.
(Assignee)

Comment 10

16 years ago
All instances of detaint_string() were replaced in the original patch
with either detaint_natural() or trick_taint() (which, I just noticed,
is named poorly--it should be trick_detaint() or detaint_trick()).

I grepped for other uses of detaint_string(), and there were none
outside the original patch, so I figured I would get rid of it in
favor of the newer functions.

I focused on only grabbing the "taint"-related changes from that
patch.  The rest of the changes were left out.
Assignee: justdave → ddkilzer
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED

Comment 11

16 years ago
I understand that you're trying to make it match what the original patch was,
but my goal is to keep the security patch (to version 2.14.1) as small as
possible.  To that end, I'd rather see just the addition of detaint_natural()
than all the changes needed in processmail, etc. to rename detaint_string() to
trick_taint().

Also, trick_taint() is the correct name for the long term as the only reason to
call that routine is to literally trick the tainting mechenism to think
something is safe (because you know there's no way it can't be) even though its
logic tells it that the string isn't safe.
Whiteboard: 2.14.1 blocker ? → 2.14.1 blocker
(Assignee)

Comment 12

16 years ago
Created attachment 63274 [details] [diff] [review]
Patch v.2

The kindler, gentler version of Patch v.1 (attachment 63263 [details] [diff] [review]) with
only the detaint_natural() subroutine added as well as the
attribution line for Bradley Baetz.  Obsoleted first patch.
Attachment #63263 - Attachment is obsolete: true
Comment on attachment 63274 [details] [diff] [review]
Patch v.2

Looks good. Works as expected.
Attachment #63274 - Flags: review+
Patch for fix checked in 2001/01/03 on -rBUGZILLA-2_14_1-BRANCH. Resolving as FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Opening security bugs for which fixes have appeared in official bugzilla
release.  As per justdave and his posse.
Group: security?
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.