New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 624213 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Long OOO (go/where-is-mgiuca)
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Address bar RTL character spoofing on Mac

Project Member Reported by mgiuca@chromium.org, Jun 29 2016

Issue description

Spinning off from  Issue 609680 . The same vulnerability exists on Mac.

VULNERABILITY DETAILS
Visit:
http://127.0.0.1/%D8%A7/example.org

It is rendered as:
example.org/‭ا‬/127.0.0.1

This is a spoofing vulnerability because it looks like you are on example.org.

The RTL character U+FE70 ARABIC FATHATAN ISOLATED FORM combined with numerical IP address causes the Omnibox text to be treated as an RTL paragraph.

VERSION
(I tested it awhile ago, not sure of the exact version. Will re-verify but I think it's going to affect all versions of Chrome and Mac OS.)

REPRODUCTION CASE
See above.
 

Comment 1 by palmer@chromium.org, Jun 29 2016

Labels: Security_Severity-Medium Security_Impact-Stable M-53
Project Member

Comment 2 by sheriffbot@chromium.org, Jun 29 2016

Labels: -Pri-2 Pri-1
Cc: rbsoulhu...@gmail.com
+rbsoulhunter who previously reported this vulnerability and has been in contact with me about it.
Some stuff I learned about Cocoa (for my reference):

- NSTextField is an NSControl.
- NSControl has a currentEditor property that is an NSTextView (for some reason it returns an NSText but can be cast to NSTextView).
- NSTextView has a textStorage property that is an NSTextStorage.
- NSTextStorage is an NSMutableAttributedString.
- NSMutableAttributedString has a setBaseWritingDirection method that *might* be helpful.

This is nuts. WIP CL (not compiled; having trouble building on Mac right now):
https://codereview.chromium.org/2126023003

Not a Mac developer at all, #ihavenoideawhatimdoing.

Comment 5 by mgiuca@chromium.org, Jul 13 2016

Cc: justincohen@chromium.org rohitrao@chromium.org
I would appreciate, if someone can look into this too, I am holding my writeup. 

Comment 7 by mgiuca@chromium.org, Jul 21 2016

Labels: -M-53 M-54
Status: Fixed (was: Assigned)
Not sure why Bugdroid hasn't posted here, but the fix landed:
https://codereview.chromium.org/2126023003
Committed: https://crrev.com/05baf795316dd430afbd79ee915187bec6bf5f5d
Cr-Commit-Position: refs/heads/master@{#406791}
Project Member

Comment 8 by sheriffbot@chromium.org, Jul 21 2016

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05baf795316dd430afbd79ee915187bec6bf5f5d

commit 05baf795316dd430afbd79ee915187bec6bf5f5d
Author: mgiuca <mgiuca@chromium.org>
Date: Thu Jul 21 07:06:50 2016

Mac (Cocoa) Omnibox: Force text field to LTR context if it is a URL.

This means that URLs will be displayed in a left-to-right paragraph
context. Right-to-left runs are still rendered RTL, but will not flip
the whole URL around. For example (if "ABC" is Hebrew), this will render
"ABC.com" as "CBA.com", rather than "com.CBA".

Affects main text field and suggestions, but not non-URL search text.
Complies with RFC 3987 Section 4.1 and brings the Mac Omnibox in line
with the existing behaviour on Views and Android.

BUG= 624213 

Review-Url: https://codereview.chromium.org/2126023003
Cr-Commit-Position: refs/heads/master@{#406791}

[modify] https://crrev.com/05baf795316dd430afbd79ee915187bec6bf5f5d/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
[modify] https://crrev.com/05baf795316dd430afbd79ee915187bec6bf5f5d/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm

Labels: -M-54 Merge-Request-53 M-53
Requesting a merge to M53. (Security, long-standing issue so not strictly urgent, but good to fix ASAP.)

Comment 11 by dimu@chromium.org, Jul 26 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 26 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ba00cae40b82241c6416e7d24c4d7aca7a227c6

commit 6ba00cae40b82241c6416e7d24c4d7aca7a227c6
Author: Matt Giuca <mgiuca@chromium.org>
Date: Tue Jul 26 03:24:27 2016

Mac (Cocoa) Omnibox: Force text field to LTR context if it is a URL.

This means that URLs will be displayed in a left-to-right paragraph
context. Right-to-left runs are still rendered RTL, but will not flip
the whole URL around. For example (if "ABC" is Hebrew), this will render
"ABC.com" as "CBA.com", rather than "com.CBA".

Affects main text field and suggestions, but not non-URL search text.
Complies with RFC 3987 Section 4.1 and brings the Mac Omnibox in line
with the existing behaviour on Views and Android.

BUG= 624213 

Review-Url: https://codereview.chromium.org/2126023003
Cr-Commit-Position: refs/heads/master@{#406791}
(cherry picked from commit 05baf795316dd430afbd79ee915187bec6bf5f5d)

Review URL: https://codereview.chromium.org/2183723002 .

Cr-Commit-Position: refs/branch-heads/2785@{#353}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/6ba00cae40b82241c6416e7d24c4d7aca7a227c6/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
[modify] https://crrev.com/6ba00cae40b82241c6416e7d24c4d7aca7a227c6/chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm

Labels: -Hotlist-Merge-Approved
Labels: Release-0-M53
Labels: CVE-2016-5167
Project Member

Comment 16 by sheriffbot@chromium.org, Oct 27 2016

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: CVE_description-submitted

Sign in to add a comment