incorrect values may be passed to variables if regular expressions return no value

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
13 years ago
3 years ago

People

(Reporter: LpSolit, Unassigned)

Tracking

Dependency tree / graph

Details

Attachments

(1 obsolete attachment)

(Reporter)

Description

13 years ago
Bug 297928 fixed the problem about unreliable $1 for detaint_natural,
detaint_signed and trick_taint routines only. We have to fix this problem in the
rest of the code too.
(Reporter)

Updated

13 years ago
Flags: blocking2.20?
Flags: blocking2.18.2?
Target Milestone: --- → Bugzilla 2.18

Comment 1

13 years ago
My initial analysis of the problem finds the following locations:

  * LoadTemplate in editclassifications.cgi
  * doGroupChanges in editgroups.cgi
  * bz_lock_tables in DB/Pg.pm
  * login in Auth/Login/WWW/Env.pm
  * editwhines.cgi (wtf is up with this code)
  * checksetup.pl around line 2448 (though this seems not to be a real problem)

Search.pm may be affected: I don't understand that code.

I couldn't find anywhere else that looks like a problem. A patch to fix at least
those problems would be very welcome.
(Reporter)

Comment 2

13 years ago
what's the regexp you used? mine is "grep -rn -A 2 -B 2 --color '= $1' *" and I
get a full list of potential problems (I did not investigated).

Comment 3

13 years ago
I just grepped for $1 and looked through the results one by one. It doesn't
actually occur so often.

Comment 4

13 years ago
I realize I actually may have missed some spots for another fundamental
misunderstanding: if you have an re like /(a|b)?(.*)/ and the first item doesn't
match, $1 is also undefined. This generates some warnings, such as
Bugzilla::Search's SqlifyDate, on the line:

        if ($sign eq '+') { $amount = -$amount; }

That needs to be tested:

        if ($sign && $sign eq '+') { $amount = -$amount; }
Flags: blocking2.20?
Flags: blocking2.20+
Flags: blocking2.18.4+
Flags: blocking2.18.2?

Comment 5

13 years ago
The problem in Env.pm is completely destroying my access to New Charts, so I'm
splitting that bug off and fixing it now.
Depends on: 300403

Comment 6

13 years ago
I'm creating a new bug to handle the noted problem with editwhines.

Updated

13 years ago
Depends on: 300831

Updated

13 years ago
Whiteboard: [bug awaiting patch]

Comment 7

13 years ago
JIMHO
1. LoadTemplate in editclassifications.cgi is safe: there are no way to execute
this sub with user-modified or undef action value. Only pre-set ones.
2. editgroups.cgi - It's trivial to fix. Patch is on it's way...
3. editwhines.cgi - as well as #2 

Comment 8

13 years ago
Created attachment 189893 [details] [diff] [review]
Simple patch for editgroups.cgi and editwhines.cgi

If I understand the problem right, simply adding "|| ''" will help to handle
undef $1 problem. Also, I've changed "=~ $1" to "= $1", but I'm not sure was it
right or not. Will also ask Erik for comments on last issue.

Comment 9

13 years ago
Comment on attachment 189893 [details] [diff] [review]
Simple patch for editgroups.cgi and editwhines.cgi

Oops... Missed already posted patch for bug 300831. Marking my one as obsolete.
Attachment #189893 - Attachment is obsolete: true

Comment 10

13 years ago
justdave -- I think that this isn't a blocker, per se, unless we can prove that
it actually causes some specific bug.
ok.
Flags: blocking2.20+
Flags: blocking2.18.4+
Target Milestone: Bugzilla 2.18 → Bugzilla 2.22
(Reporter)

Updated

13 years ago
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
(Reporter)

Comment 12

12 years ago
We are freezing the code for 3.0 in two weeks and we don't expect this bug to be fixed on time.
Target Milestone: Bugzilla 3.0 → ---
(Reporter)

Comment 13

3 years ago
Specific problems will be filed separately.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
(Reporter)

Updated

3 years ago
Whiteboard: [bug awaiting patch]
(Reporter)

Updated

3 years ago
Depends on: 1234977
You need to log in before you can comment on or make changes to this bug.