Closed
Bug 509794
Opened 16 years ago
Closed 16 years ago
Crash if setting a flag with Unicode characters in the name
Categories
(Bugzilla :: Attachments & Requests, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.4
People
(Reporter: pzn, Assigned: LpSolit)
References
Details
Attachments
(2 files)
706 bytes,
text/plain
|
Details | |
804 bytes,
patch
|
bbaetz
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; pt-BR; rv:1.9.0.13) Gecko/2009080315 Ubuntu/9.04 (jaunty) Firefox/3.0.5, Ant.com Toolbar 1.1
Build Identifier: Bugzilla 3.4.1
bugzilla crashes with the following message:
Software error: Insecure dependency in parameter 8 of DBI::db=HASH(0x92d3624)->do method call while running with -T switch at Bugzilla/Flag.pm line 636, <fh00001163_r5.diff> line 1. For help, please send mail to the webmaster
the conditions to trigger the bug are: in the "new attachment window"
- chose a file
- turn on "is a patch"
- turn on "obsoletes" of some previous file
- set the flag "revision" to "?" and select an username to review the patch
- write a comment
if you do ALL this at the same time, it triggers the bug.
if I do in some steps (add the attach, then enter it to edit the revision, obsolete, ...) it does not trigger the bug.
Reproducible: Always
Comment 1•16 years ago
|
||
Does your flag's name have any Unicode characters or a Unicode space/hyphen in it?
Comment 2•16 years ago
|
||
Also, can you reproduce this at http://landfill.bugzilla.org/bugzilla-3.4-branch/ ?
Reporter | ||
Comment 3•16 years ago
|
||
could not reproduce at landfill.
my 3.4.1 was downloaded and I applied patches to translation.
do you think that could be triggered by the template patches?
Comment 4•16 years ago
|
||
(In reply to comment #3)
> could not reproduce at landfill.
> my 3.4.1 was downloaded and I applied patches to translation.
> do you think that could be triggered by the template patches?
Yeah, that could be. Do you mean that you actually modified the templates, or that you just installed a publicly-available localization pack?
Reporter | ||
Comment 5•16 years ago
|
||
there is no public-available template for my language. I did it by myself.
yes, my flag has unicode chars in its name.
Comment 6•16 years ago
|
||
(In reply to comment #5)
> yes, my flag has unicode chars in its name.
Okay, I suspect that that is the problem, having seen this in other places and having resolved it by removing the Unicode characters.
We should investigate why trick_taint isn't detainting utf8 flag names, or why in the world DBI would think that they were tainted to begin with.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: qawanted
OS: Linux → All
Hardware: x86 → All
Summary: crash if adding an attachment with flags and comments → Crash if setting a flag with Unicode characters in the name
Target Milestone: --- → Bugzilla 3.4
Reporter | ||
Comment 7•16 years ago
|
||
what is weird, is that I can set the flag in other conditions.
just setting the flag is OK.
setting the flag + add comments at the same time, triggers the bug.
----
I entered in the definition of the flag and removed all unicode chars from its name and from its description.
Now bugzilla works well in the same conditions. no errors anymore.
![]() |
Assignee | |
Comment 8•16 years ago
|
||
(In reply to comment #0)
> Software error: Insecure dependency in parameter 8 of
> DBI::db=HASH(0x92d3624)->do method call while running with -T switch at
> Bugzilla/Flag.pm line 636
This is:
$dbh->do('INSERT INTO bugs_activity
(bug_id, attach_id, who, bug_when, fieldid, removed, added)
VALUES (?, ?, ?, ?, ?, ?, ?)',
undef, ($bug_id, $attach_id, Bugzilla->user->id,
$timestamp, $field_id, $removed, $added));
So I guess param 8 is $added, which is considered tainted.
![]() |
Assignee | |
Comment 9•16 years ago
|
||
OK, I can reproduce the bug locally in 3.4.1 under the following conditions:
1. The flag must have Unicode in it;
2. You must set the flag to "?" + have a requestee;
3. Add a comment (nothing special in it, pure ASCII is enough).
You can also trigger the same error if you replace 2. by:
2'. The flag was set to "?" with a requestee, and you set the flag to "+", "-" or cancel it.
The easiest workaround is probably to trick_taint() $removed and $added before passing them to DBI. I don't like it very much as it shouldn't be tainted, but it's safe as we use placeholders.
Assignee: attach-and-request → LpSolit
Flags: blocking3.4.2+
Keywords: qawanted
Version: unspecified → 3.4
![]() |
Assignee | |
Comment 10•16 years ago
|
||
I found what taints data:
foreach (@$removed, @$added) { s/^[^:]+:// } in Flag.pm at line 633.
Right before this line, nothing is tainted (as expected). Right after this line, data having Unicode + requestee is tainted.
![]() |
Assignee | |
Comment 11•16 years ago
|
||
I tried to write a reduced testcase, but the string isn't tainted after the regexp.
Comment 12•16 years ago
|
||
That sounds like a bug in Perl, to me.
![]() |
Assignee | |
Comment 13•16 years ago
|
||
(In reply to comment #12)
> That sounds like a bug in Perl, to me.
No doubt about that. But without a reduced testcase, there is no chance to report the issue upstream. :(
For now, I will simply call trick_taint().
Comment 14•16 years ago
|
||
I've tried reducing this (on landfill). (Below is a braindump)
* The comment has to be there.
* Whats triggering this is the creation of the notification mail to the requestee (note - not sending the mail) (Bugzilla::Flag::notify)
* The minimal template to trigger this is:
[% USE Bugzilla %]
[% IF Bugzilla.cgi.param("comment") && Bugzilla.cgi.param("comment").length > 0 %]
[% END %]
* This must be an if statement, and both parts of the conditional must be there - removing the IF, or having only one part has no problems.
* This occurs even if I don't pass any params into the inner template processing
* Using the pure-perl XS stash avoids the problem
* Can't reproduce from the command line
* Can't reproduce on my F11 perl 5.10 machine
<much time passes>
Minmimal non-bugzilla test case attached
![]() |
Assignee | |
Comment 15•16 years ago
|
||
(In reply to comment #14)
> * The minimal template to trigger this is:
> [% USE Bugzilla %]
> [% IF Bugzilla.cgi.param("comment") && Bugzilla.cgi.param("comment").length > 0
> %]
> [% END %]
Yes, I found that too. And you can work around this problem if you write:
[% comment = Bugzilla.cgi.param("comment") %]
[% IF comment.length %]
which is what I did locally. I think I will go with this solution, so that we don't detaint originally tainted data in Flag.pm.
Comment 16•16 years ago
|
||
I can't explain why CGI affects this test case. You need to pass |comment=test| as a param to the script for it to fail.
![]() |
Assignee | |
Comment 17•16 years ago
|
||
This fixes the problem and is safe.
Attachment #394038 -
Flags: review?(bbaetz)
Comment 18•16 years ago
|
||
https://rt.cpan.org/Ticket/Display.html?id=48654
Can we wait for a reply - I'm worried that we're just pushing the problem to show up somewhere else.
![]() |
Assignee | |
Comment 19•16 years ago
|
||
(In reply to comment #18)
> Can we wait for a reply - I'm worried that we're just pushing the problem to
> show up somewhere else.
It's probably a bug in Perl itself. I can reproduce the taint issue with Perl 5.10.0, but I cannot reproduce with Perl 5.10.1 RC1. In both cases do I use TT 2.22. So it doesn't look like a TT bug.
![]() |
Assignee | |
Comment 20•16 years ago
|
||
bbaetz: ping?
Updated•16 years ago
|
Attachment #394038 -
Flags: review?(bbaetz) → review+
Comment 21•16 years ago
|
||
Comment on attachment 394038 [details] [diff] [review]
patch, v1
No update on the TT RT issue, so r=bbaetz if you add a (TT) comment above the changed line pointing to this bug. (Make sure that that doesn't add an extra output line, though)
![]() |
Assignee | |
Updated•16 years ago
|
Flags: approval3.4+
Flags: approval+
![]() |
Assignee | |
Comment 22•16 years ago
|
||
tip:
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v <-- email.txt.tmpl
new revision: 1.23; previous revision: 1.22
done
3.4.1:
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v <-- email.txt.tmpl
new revision: 1.21.2.1; previous revision: 1.21
done
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•