Closed
Bug 297940
Opened 19 years ago
Closed 9 years ago
incorrect values may be passed to variables if regular expressions return no value
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: LpSolit, Unassigned)
References
Details
Attachments
(1 obsolete file)
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•19 years ago
|
Flags: blocking2.20?
Flags: blocking2.18.2?
Target Milestone: --- → Bugzilla 2.18
Comment 1•19 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•19 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•19 years ago
|
||
I just grepped for $1 and looked through the results one by one. It doesn't actually occur so often.
Comment 4•19 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; }
Updated•19 years ago
|
Flags: blocking2.20?
Flags: blocking2.20+
Flags: blocking2.18.4+
Flags: blocking2.18.2?
Comment 5•19 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•19 years ago
|
||
I'm creating a new bug to handle the noted problem with editwhines.
Updated•19 years ago
|
Whiteboard: [bug awaiting patch]
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
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 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•19 years ago
|
||
justdave -- I think that this isn't a blocker, per se, unless we can prove that it actually causes some specific bug.
Comment 11•19 years ago
|
||
ok.
Flags: blocking2.20+
Flags: blocking2.18.4+
Target Milestone: Bugzilla 2.18 → Bugzilla 2.22
Reporter | ||
Updated•19 years ago
|
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
Reporter | ||
Comment 12•18 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•9 years ago
|
||
Specific problems will be filed separately.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Updated•9 years ago
|
Whiteboard: [bug awaiting patch]
You need to log in
before you can comment on or make changes to this bug.
Description
•