[SECURITY] Dangerous control characters allowed in Bugzilla text

RESOLVED FIXED in Bugzilla 4.0

Status

()

Bugzilla
User Interface
--
minor
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: manishearth, Assigned: manishearth)

Tracking

({sec-low, wsec-other})

unspecified
Bugzilla 4.0
sec-low, wsec-other
Dependency tree / graph
Bug Flags:
approval +
approval4.4 +
blocking4.4.3 +
approval4.2 +
blocking4.2.8 +
approval4.0 +
blocking4.0.12 +
sec-bounty +

Details

(Whiteboard: [reporter-external])

Attachments

(5 attachments, 8 obsolete attachments)

369 bytes, text/html
Details
2.57 KB, patch
glob
: review+
Details | Diff | Splinter Review
2.67 KB, patch
glob
: review+
Details | Diff | Splinter Review
2.58 KB, patch
glob
: review+
Details | Diff | Splinter Review
2.59 KB, patch
glob
: review+
Details | Diff | Splinter Review
(Assignee)

Description

3 years ago
Bugzilla, currently, does not block control characters from being entered in code blocks.

Since it's a bug tracking website, we will have people passing around bash scripts. And code to be entered into an editor.

Now, just take a look at the following bash script:

#Creative Commons Attribution-ShareAlike / Author Manishearth
echo something
#Do more things


It looks pretty harmless.

If you copy paste it into an editor (do NOT copy paste it into the terminal), it will run `rm -r /`, as the first line, while it *looks* like an innocuous comment, is really one of the easiest ways to lose most of your data on a Linux system. (This works on Windows too, though the exact implementation would be different)

This works because the first line contains a lot of ^H characters that mask the real meaning of the 

While I don't have a testcase for it, this would work for Vim too. Many users here use Vim, and using a combination of the ESC (^[) character, one could theoretically execute some shell command (using the Vim command `:!` to run shell commands). This can again be hidden away in a larger comment or line of code using ^H.

There is a case for fixing this in the browser itself. I am filing separate bugs for that.
(Assignee)

Comment 1

3 years ago
(somehoe submitted an older draft of the report)

Just a note: in the post where I said "It will run `rm -r /`", I meant that it will display `rm -r /`. It would have run the code had you copied it into the terminal prompt directly, which I hope you didn't.

And the testcase only works in a terminal-based editor, like nano or vim. Most GUI editors render or hide control characters. Of course, the *actual* target is the prompt itself, but for viewing purposes you need a terminal based thing.


As for the unfinished sentence about ^H, I'm saying that there are a lot of ^H characters which the terminal interprets as a backspace (which it should), and deletes stuff. By backspacing the right characters, I have reduced the harmless comment into a harmful command.
(Assignee)

Comment 2

3 years ago
Reported bug 968584, asking for this to be fixed in FireFox as a whole by rendering such characters. Not sure if it'll gain traction, rendering unprintable things sounds a bit strange to me.
(Assignee)

Comment 3

3 years ago
Which security flag should this get?

sec-moderate states "Client bugs that might have high or critical results but require the user perform unusual or complex actions to trigger." -- I don't know if copying code into Vim or bash on a bug tracker is "unusual", I do it quite often.

sec-high states "requiring no more than normal browsing actions." -- which this doesn't. It does, however, give the attacker the ability to execute pretty much anything on the local machine since a lot of stuff can be hidden in seemingly innocuous code. Especially if you know Perl.
See Also: → bug 968584
the "harmfullness" of characters is wholly dependant on where they are being pasted, making true sanitisation within bugzilla tricky.  it took me a while to reproduce this issue because my terminal emulator was stripping ^H when pasting.

we could strip all control chars when rendering the comment with s/(?![\t\r\n])[[:cntrl:]]//g
(Assignee)

Comment 5

3 years ago
(In reply to Byron Jones ‹:glob› from comment #4)
> the "harmfullness" of characters is wholly dependant on where they are being
> pasted, making true sanitisation within bugzilla tricky.  it took me a while
> to reproduce this issue because my terminal emulator was stripping ^H when
> pasting.


True. Though the immediately harmful (and easiest to abuse) ones are ^H and ESC. 


As far as I can tell this is standard behavior on Linux distros; control chars are very much a part of the system clipboard. Which makes sense in a way, because ^M (line break) is a control char as well, along with some other things which you *do* want to be able to copy.

I was able to reproduce this on gnome-terminal, xterm, and terminator on Ubuntu 13.04. I'm reasonably certain it will work on Fedora as well.

IIRC this works for PowerShell and cmd as well, though I don't know about PuTTY (will verify later)



> we could strip all control chars when rendering the comment with
> s/(?![\t\r\n])[[:cntrl:]]//g


That regex ought to work.
(Assignee)

Comment 6

3 years ago
As far as I can tell, we just have to add a regex match to line 247[1]


Though MDN probably has this issue too (and there is a lot more code to copy there), we should look into that.

 [1]: http://mxr.mozilla.org/bugzilla/source/process_bug.cgi#247
(In reply to Manish Goregaokar [:manishearth] from comment #6)
> As far as I can tell, we just have to add a regex match to line 247[1]

no, if we do do this, we should do it in the web template at display time, and not touch the data stored.
(Assignee)

Comment 8

3 years ago
Ah, that works too. Though there seems to be only one entry point for comments in the code, but multiple exits.
(Assignee)

Comment 9

3 years ago
Created attachment 8371259 [details]
HTML testcase

Also hosted at http://manishearth.anapnea.net/testcontrol.html
(Assignee)

Updated

3 years ago
Attachment #8371259 - Attachment mime type: text/plain → text/html
> Though there seems to be only one entry point for comments in the code.

your guess isn't accurate - web services can also add comments.  if we were to strip control char prior to being inserted into the database, the comment class would be the correct location, not at a boundary (we'd also need to detect and fix any existing comments via checksetup).

fixing it with a template filter addresses the issue at hand with minimal risk. an unaltered form of the comment will still be available via alternative calls (xml.cgi, rpc, rest, ..), and it means if the fix is incorrect (eg. utf8 issue), then no data will be lost.

> Created attachment 8371259 [details]
> HTML testcase

we should *not* be sanitising attachment content in any way.
(Assignee)

Comment 11

3 years ago
(In reply to Byron Jones ‹:glob› from comment #10)
> fixing it with a template filter addresses the issue at hand with minimal risk.

Makes sense. First time I'm looking at Bugzilla code, I'm getting lost :p

> > Created attachment 8371259 [details]
> > HTML testcase
> 
> we should *not* be sanitising attachment content in any way.

Of course. I just attached HTML testpages to both of the bugs because it's useful to have a place to copy this from.

Though I just realized that I already had a testcase in the question itself. Oops.
(Assignee)

Comment 12

3 years ago
Created attachment 8371542 [details] [diff] [review]
Patch for comments.html.tmpl

Patched it with a second FILTER.

Not tested, and I'm really not good with Perl.
Attachment #8371542 - Flags: review?(glob)
(Assignee)

Comment 13

3 years ago
Are there any other places where comments are exposed in the templates? Doesn't look like it, but I'm not familiar with the code.
(Assignee)

Updated

3 years ago
Assignee: ui → manishearth
Flags: sec-bounty?
Whiteboard: [reporter-external]

Comment 14

3 years ago
(In reply to Byron Jones ‹:glob› from comment #4)
> we could strip all control chars when rendering the comment with
> s/(?![\t\r\n])[[:cntrl:]]//g

I'm not sure what you want to do with (?![\t\r\n]), but http://perldoc.perl.org/perlre.html#Look-Around-Assertions says that:

"If you are looking for a "bar" that isn't preceded by a "foo", /(?!foo)bar/ will not do what you want."

So this doesn't look like the right regexp.
(Assignee)

Comment 15

3 years ago
(In reply to Frédéric Buclin from comment #14)

> "If you are looking for a "bar" that isn't preceded by a "foo", /(?!foo)bar/
> will not do what you want."

But we're not looking for a "bar" that isn't preceded by a foo. We're looking for a "bar" ([[:cntrl:]) that isn't a "foo" (the three "special" control characters). From the same page:

> That's because the (?!foo) is just saying that the next thing cannot be "foo"--and it's not, it's a "bar", so "foobar" will match.

Here, we _do_ want the next thing to not be "foo". He's using a lookahead as a negative regex.

Another way to do it would be to use the regex `(?[ [[:cntrl:]] - [\t\r\n] ])`. I think.
(Assignee)

Updated

3 years ago
See Also: → bug 969084
Comment on attachment 8371542 [details] [diff] [review]
Patch for comments.html.tmpl

we should also filter control characters from bugmail (both plain text and html), in order to provide some protection to email clients which don't filter.

please create a new filter in Bugzilla/Template.pm and use it in the bug/comments.html.tmpl and the email/bugmail.{txt,html}.tmpl files.

if you aren't sure how to do this, i'm happy to create the patch for you.
Attachment #8371542 - Flags: review?(glob) → review-

Comment 17

3 years ago
(In reply to Byron Jones ‹:glob› from comment #16)
> we should also filter control characters from bugmail

If an attacker wants to abuse an email client, he doesn't need Bugzilla to do this. Also, if you follow this path, you would have to filter *all* possible field values everywhere, because these characters could be in the bug summary, bug comment, username, status whiteboard, attachment name, attachment description, etc... And as a reviewer, I guess you do like me and copy and paste the bug summary and the username into your commit message, so these fields would have to be filtered too. Why not simply fix the HTML filter to remove these characters for us? We really don't need nor want another filter (else you could have things like [% foo FILTER js FILTER html FILTER bar %] and this is prone to errors).

Also, validators should automatically remove these control characters, e.g. by using clean_text() instead of trim(). This could be done in a separate bug.
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 18

3 years ago
(In reply to Frédéric Buclin from comment #17)
> (In reply to Byron Jones ‹:glob› from comment #16)

> Also, if you follow this path, you would have to filter *all*
> possible field values everywhere, because these characters could be in the
> bug summary, bug comment, username, status whiteboard, attachment name,
> attachment description, etc...

Technically no, the only place people copy code from are the comments/description. And possibly from patch diffs (which we probably don't want to touch).

The current patch fixes it for the description and comments.

You're right about the bugmail thing. Most mail clients effectively handle control chars anyway, and for one that doesn't, I could just spoof the mail (unless they're using signed mail)

Comment 19

3 years ago
(In reply to Manish Goregaokar [:manishearth] from comment #18)
> Technically no, the only place people copy code from are the
> comments/description.

Technically, yes, these fields can contain control characters! I doubt you know what the habits of other users are. And as I said in my previous comment, reviewers (I am one of them) copy and paste several data into the commit message. So the right place to fix this problem is the HTML filter itself.
Severity: normal → minor
(Assignee)

Comment 20

3 years ago
(In reply to Frédéric Buclin from comment #19)
> And as I said in my previous
> comment, reviewers (I am one of them) copy and paste several data into the
> commit message. So the right place to fix this problem is the HTML filter
> itself.


Hah, good point. Alright, I'll look into the HTML filter later today.
(Testing that SecureMail is working for me...)

Gerv
(Assignee)

Comment 22

3 years ago
(In reply to Frédéric Buclin from comment #17)
> Why not simply fix the HTML  filter to remove these characters for us?

Looks like there isn't any HTML filter being applied on output for the comments, I think the data is entity-ified on input.

http://mxr.mozilla.org/bugzilla/source/template/en/default/bug/comments.html.tmpl#209 


So FILTER html-ing this may end up with double encoding.

In addition, in some cases I see html_quote (FILTER html) being applied _with_ clean_text() (which trims out extra whitespace and control chars) and sometimes it isn't. Of course, I can still add a `=~ s/[\x00-\x1F\x7F]+/ /g` to the HTML filter, that shouldn't be a problem, but it's not being used everywhere the HTML filter is used.

So, what I can do is:

 - Update the HTML filter to strip control chars
 - Update the comment and comment email templates to strip control chars (using a different filter created in Template.pm)

I'm not sure if I can bring uniformity and have them all use the same filter as there seems to be a lot of nonuniformity in usage.

Any suggestions?
Flags: needinfo?(LpSolit)
(Assignee)

Comment 23

3 years ago
Created attachment 8373983 [details] [diff] [review]
strip control chars in html filter, add new controlchars filter, apply to comments. and bugmail templates.

This patch moves the filter to a different method, and uses that method in comments.html.tmpl and bugmail.*.tmpl.


It also updates the html filter (FILTER html is aliased to html_quote from Util.pm) so that it strips out control characters.
Attachment #8371542 - Attachment is obsolete: true
Attachment #8373983 - Flags: review?(glob)
Comment on attachment 8373983 [details] [diff] [review]
strip control chars in html filter, add new controlchars filter, apply to comments. and bugmail templates.

Review of attachment 8373983 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for taking the time to work through this patch :)

> You're right about the bugmail thing. Most mail clients effectively handle
> control chars anyway, and for one that doesn't, I could just spoof the mail
> (unless they're using signed mail)

this whole bug is about protecting users from clients which don't do the right thing (in this case terminal emulators and web browsers).

::: Bugzilla/Template.pm
@@ +681,5 @@
>                  return encode_base64($data);
>              },
> +
> +            # Strips out control characters
> +            controlchars => sub {

i think that name should be clearer: like strip_control_chars

@@ +683,5 @@
> +
> +            # Strips out control characters
> +            controlchars => sub {
> +                my ($data) = @_;
> +                $data =~ s/[\x00-\x1F\x7F]+/ /g;

we shouldn't be removing \t \r or \n.

we should be _removing_ the characters, not replacing them with a space. this mirrors how they are currently displayed in the browsers.

for clarity please use :cntrl: not a character range.

::: Bugzilla/Util.pm
@@ +71,5 @@
>      $var =~ s/"/"/g;
>      # Obscure '@'.
>      $var =~ s/\@/\@/g;
> +    # Removing control characters, bug 968576
> +    $var =~ s/[\x00-\x1F\x7F]+/ /g;

no need to mention a bug number, source control's blame does that for us.
the same comments above apply here.
Attachment #8373983 - Flags: review?(glob) → review-

Comment 25

3 years ago
Comment on attachment 8373983 [details] [diff] [review]
strip control chars in html filter, add new controlchars filter, apply to comments. and bugmail templates.

>=== modified file 'template/en/default/bug/comments.html.tmpl'

>-  [%- comment_text FILTER quoteUrls(bug, comment) -%]
>+  [%- comment_text FILTER quoteUrls(bug, comment) FILTER controlchars -%]

There is no need for a new filter. quoteUrls() already calls html_quote(), and html_quote() already removes control characters.



>=== modified file 'template/en/default/email/bugmail.html.tmpl'

>-        <pre>[% comment.body_full({ wrap => 1 }) FILTER quoteUrls(bug, comment, to_user) %]</pre>
>+        <pre>[% comment.body_full({ wrap => 1 }) FILTER quoteUrls(bug, comment, to_user) FILTER controlchars %]</pre>

Same here.


If we decide to also filter plain text emails, then you also have to fix flagmail.txt, which also displays the comment.



Not sure how you generated the patch (bzr?), but PatchReader is unable to parse it correctly because the file name and modification date in headers are separated by spaces instead of tabs.
Attachment #8373983 - Flags: review-

Updated

3 years ago
Flags: needinfo?(LpSolit)
(Assignee)

Comment 26

3 years ago
Created attachment 8374213 [details] [diff] [review]
Incorporate comment 24, 25

Alright, I think I fixed that.

@Frederic I generated it with bzr, however I did so on a separate server* and lazily copied the text over. This time I've scp'd it instead, the spaces should work now. Apologies for the inconvenience.


*University proxy was giving me issues with bzr and while I can co over http://, it was really slow. So I sshed out and did the editing.
Attachment #8373983 - Attachment is obsolete: true
Attachment #8374213 - Flags: review?(glob)
Comment on attachment 8374213 [details] [diff] [review]
Incorporate comment 24, 25

r=glob :)
Attachment #8374213 - Flags: review?(glob) → review+
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?

Updated

3 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Comment 28

3 years ago
Just realized something -- do support multibyte characters on Bugzilla?

If so, this fix may break this. Some multibyte encodings might use the control char byterange.

As far as I can tell, this isn't the case; since bugzilla is utf8 which preserves all ascii (unless the user is allowed to choose an encoding?). Just checking.
Since this isn't an attack on bugzilla itself or browsers this really can't get more than a sec-low. The results could be catastrophic for a user if you convince them to cut and paste into a command line, but that is definitely not "normal browsing" or normal use of bugzilla.
Keywords: sec-low, wsec-other

Comment 30

3 years ago
(In reply to Manish Goregaokar [:manishearth] from comment #28)
> Just realized something -- do support multibyte characters on Bugzilla?

If the utf8 parameter is turned on, then the DB uses UTF8 as encoding. If the parameter is left off, then you have no guarantee what the encoding is.

(19:43:04) dveditz: unless the DB is guaranteed to be utf-8 then the regexp is going to fail
(19:43:35) LpSolit: if the utf8 parameter is turned on, then yes, the DB is utf8-encoded
(19:43:41) dveditz: You'd have to put a banner on bugzilla "Not for use in Asian languages"
(19:43:57) dveditz: if it's off this regexp is trouble


Maybe only run the regexp if Bugzilla->params->{utf8} is true?
(Assignee)

Comment 31

3 years ago
Created attachment 8375715 [details] [diff] [review]
Ignore non-utf8 encodings

This patch only does the substitution for utf8.

(Not obsoleting the first one since I'm not yet sure if we need this)

Comment 32

3 years ago
Comment on attachment 8375715 [details] [diff] [review]
Ignore non-utf8 encodings

>=== modified file 'Bugzilla/Template.pm'

>+                # Only run for utf8 to avoid issues with other multibyte encodings that may be reassigning meaning to ascii characters.

This line is too long.



>=== modified file 'Bugzilla/Util.pm'

>     $var =~ s/\@/\&#64;/g;
>-

Nit: don't remove this newline, for readability.


>+        # Remove control characters if the encoding is utf8
>+        # Othr multibyte encodings may be using this range; so ignore if not utf8

Missing period at the end of sentences. Also s/Othr/Other/.

Comment 33

3 years ago
Note also that this patch doesn't apply to branches, so a backport will be needed.
(Assignee)

Comment 34

3 years ago
Created attachment 8375739 [details] [diff] [review]
Ignore non-utf8 encodings v2

Fixed.
Attachment #8375715 - Attachment is obsolete: true

Comment 35

3 years ago
That's the same patch.
(Assignee)

Comment 36

3 years ago
Created attachment 8375744 [details] [diff] [review]
Ignore non-utf8 encodings v2

*facepalm*

Got it this time, hopefully.
Attachment #8375739 - Attachment is obsolete: true
(In reply to Frédéric Buclin from comment #30)
> (In reply to Manish Goregaokar [:manishearth] from comment #28)
> If the utf8 parameter is turned on, then the DB uses UTF8 as encoding. If
> the parameter is left off, then you have no guarantee what the encoding is.

I was somehow convinced that the current version forced UTF8... I thought that param went away after a few versions when we thought people had enough time to migrate.  But sure enough I still see it on bugzilla-tip.  I assume we're holding off on approval until we're ready to roll with the security advisory and whatnot.

Comment 38

3 years ago
(In reply to Dave Miller [:justdave] (justdave@bugzilla.org) from comment #37)
> I was somehow convinced that the current version forced UTF8... I thought
> that param went away after a few versions when we thought people had enough
> time to migrate.  But sure enough I still see it on bugzilla-tip.

We don't enforce it yet, see bug 902395.


> we're holding off on approval until we're ready to roll with the security
> advisory and whatnot.

I don't think this bug needs a security advisory. It's more a security enhancement than a bug fix. What we need are backports, though.
Flags: sec-bounty? → sec-bounty+

Comment 40

3 years ago
@glob: see comment 30. I think attachment 8374213 [details] [diff] [review] should be r- in favor of attachment 8375744 [details] [diff] [review].

@Manish: we need backports for 4.x, see comment 33. Do you want to write the patches yourself or should we?

Updated

3 years ago
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?
(Assignee)

Comment 41

3 years ago
(In reply to Frédéric Buclin from comment #40)
> @glob: see comment 30. I think attachment 8374213 [details] [diff] [review]
> should be r- in favor of attachment 8375744 [details] [diff] [review].
> 
> @Manish: we need backports for 4.x, see comment 33. Do you want to write the
> patches yourself or should we?

I can do it if you have some pointers to where I can get the code for a specific release (tags?) and what I need to do with it. I assume I just need to download the older code and make the same changes, right?

Comment 42

3 years ago
bzr co bzr://bzr.mozilla.org/bugzilla/4.4

Same with 4.2 and 4.0 (if the patch for 4.4 doesn't apply there). :)
(Assignee)

Comment 43

3 years ago
(In reply to Frédéric Buclin from comment #42)
> bzr co bzr://bzr.mozilla.org/bugzilla/4.4
> 
> Same with 4.2 and 4.0 (if the patch for 4.4 doesn't apply there). :)

Looks like flagmail.txt.tmpl doesn't exist in 4.2. Should I just skip it?
(Assignee)

Comment 44

3 years ago
Never mind, figured it out.

Comment 45

3 years ago
(In reply to Manish Goregaokar [:manishearth] from comment #43)
> Looks like flagmail.txt.tmpl doesn't exist in 4.2. Should I just skip it?

It's named template/en/default/request/email.txt.tmpl
(Assignee)

Comment 46

3 years ago
Created attachment 8381034 [details] [diff] [review]
Backport: 4.4
Attachment #8381034 - Flags: review?
(Assignee)

Updated

3 years ago
Attachment #8381034 - Attachment description: Bac → Backport: 4.4
(Assignee)

Comment 47

3 years ago
Created attachment 8381036 [details] [diff] [review]
Backport: 4.2
Attachment #8381036 - Flags: review?(LpSolit)
(Assignee)

Comment 48

3 years ago
The patch for 4.2 also applies to 4.0, however the file "bugmail.txt.tmpl" seems to be new. Looking in to it now.
(Assignee)

Updated

3 years ago
Attachment #8381034 - Flags: review? → review?(LpSolit)
(Assignee)

Comment 49

3 years ago
Created attachment 8381039 [details] [diff] [review]
Backport: 4.0

I added the filter to the newchangedmail template for 4.0. Not sure if that was the right way to do it, but it looks like it applies to comment bodies.
Attachment #8381039 - Flags: review?(LpSolit)
(Assignee)

Comment 50

3 years ago
(That file was replaced with the newer bugmail templates in bug 65477)
Attachment #8374213 - Flags: review+ → review-
Comment on attachment 8375744 [details] [diff] [review]
Ignore non-utf8 encodings v2

r=glob
Attachment #8375744 - Flags: review+

Updated

3 years ago
Attachment #8374213 - Attachment is obsolete: true

Comment 52

3 years ago
Comment on attachment 8381034 [details] [diff] [review]
Backport: 4.4

I will let glob review backports as he reviewed the original one.
Attachment #8381034 - Flags: review?(LpSolit) → review?(glob)

Updated

3 years ago
Attachment #8381036 - Flags: review?(LpSolit) → review?(glob)

Updated

3 years ago
Attachment #8381039 - Flags: review?(LpSolit) → review?(glob)
Comment on attachment 8381034 [details] [diff] [review]
Backport: 4.4

bugzilla 4.4 and earlier need to run on perl v5.8, which doesn't support the "state" function.
Attachment #8381034 - Flags: review?(glob) → review-
Attachment #8381036 - Flags: review?(glob) → review-
Attachment #8381039 - Flags: review?(glob) → review-
(Assignee)

Comment 54

3 years ago
Created attachment 8381972 [details] [diff] [review]
Backport: 4.0
Attachment #8381039 - Attachment is obsolete: true
Attachment #8381972 - Flags: review?(glob)
(Assignee)

Comment 55

3 years ago
Created attachment 8381973 [details] [diff] [review]
Backport: 4.2
Attachment #8381036 - Attachment is obsolete: true
Attachment #8381973 - Flags: review?(glob)
(Assignee)

Comment 56

3 years ago
Created attachment 8381974 [details] [diff] [review]
Backport: 4.4
Attachment #8381034 - Attachment is obsolete: true
Attachment #8381974 - Flags: review?(glob)
Comment on attachment 8381972 [details] [diff] [review]
Backport: 4.0

r=glob
Attachment #8381972 - Flags: review?(glob) → review+
Comment on attachment 8381973 [details] [diff] [review]
Backport: 4.2

r=glob
Attachment #8381973 - Flags: review?(glob) → review+
Comment on attachment 8381974 [details] [diff] [review]
Backport: 4.4

r=glob
Attachment #8381974 - Flags: review?(glob) → review+

Updated

3 years ago
Flags: approval?
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?

Updated

3 years ago
Summary: Dangerous control characters allowed in Bugzilla text → [SECURITY] Dangerous control characters allowed in Bugzilla text

Updated

3 years ago
Flags: blocking4.4.3?
Flags: blocking4.2.8?
Flags: blocking4.0.12?
(Assignee)

Comment 60

3 years ago
Security advisory draft:

Class:       Other
Versions:    All
Fixed In:    4.4.3, 4.2.8, 4.0.12
Description: Currently, bugzilla does not filter out control characters. These can be used by an 
             attacker within code in comments to execute arbitrary commands when pasted 
             into a terminal-based editor or a command prompt.
References:  https://bugzilla.mozilla.org/show_bug.cgi?id=968576

Updated

3 years ago
Flags: blocking4.4.3?
Flags: blocking4.4.3+
Flags: blocking4.2.8?
Flags: blocking4.2.8+
Flags: blocking4.0.12?
Flags: blocking4.0.12+

Updated

3 years ago
Blocks: 996172
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
Flags: approval+
(Assignee)

Comment 61

3 years ago
Update to sec-adv draft: The versions should include 4.5.3

Comment 62

3 years ago
(In reply to Manish Goregaokar [:manishearth] from comment #61)
> Update to sec-adv draft: The versions should include 4.5.3

I will take care of the sec adv. This is not the single sec bug to be fixed.
(Assignee)

Comment 63

3 years ago
I know, there are two. Thanks :)

Comment 64

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   0e39097..58b92d3  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   2f385c1..d3d080f  4.4 -> 4.4

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   e5daf57..6066ff3  4.2 -> 4.2

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   bab49d4..ffb3248  4.0 -> 4.0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Comment 65

3 years ago
Security advisory sent.
Group: bugzilla-security

Updated

3 years ago
Blocks: 998323

Comment 66

3 years ago
(In reply to Manish Goregaokar [:manishearth] from comment #2)
> Reported bug 968584, asking for this to be fixed in FireFox as a whole by
> rendering such characters. Not sure if it'll gain traction, rendering

bug 868846 comment 15
You need to log in before you can comment on or make changes to this bug.