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

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security
M-5

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

Unescape according to the safe browsing spec

Project Member Reported by brettw@chromium.org, Feb 13 2009

Issue description

We don't currently unescape according to the safe browsing spec. It's my 
understanding that we use the content of a GURL directly. Safe browsing 
specifies that more stuff should be unescaped.

I think this means we're missing malicious URLs with escaped characters in 
them.
 

Comment 1 by brettw@chromium.org, Feb 17 2009

Labels: -Area-Misc Area-BrowserBackend Mstone-2.0
Status: Assigned

Comment 3 by jon@chromium.org, Apr 3 2009

Labels: JonMoved Mstone-2.1
Moving from milestone 2 to milestone 2.1.
Labels: -jonmoved
Labels: -mstone-2.1 mstone-3

Comment 6 by jon@chromium.org, Jun 5 2009

Labels: mstone4
Moving to milestone 4.  If you fix this quickly and can convince Mark it is 
important you can still get it patched into milestone 3.  Otherwise, the next 
bus is milestone 4.

Comment 7 by jon@chromium.org, Jun 5 2009

Labels: -mstone4 -mstone-3 Mstone-4
Moving to milestone 4.  If you fix this quickly and can convince Mark it is 
important you can still get it patched into milestone 3.  Otherwise, the next 
bus is milestone 4.
If anyone wants to fix this, the relevant canonicalization to follow is at http://code.google.com/p/google-safe-browsing/wiki/Protocolv2Spec section 6.1
Labels: Fixit
Labels: -private Restrict-View-SecurityTeam

Comment 11 by jon@chromium.org, Oct 1 2009

Status: Available

Comment 12 by jon@chromium.org, Oct 7 2009

Status: Untriaged
Need to reassign.
Labels: -Mstone-4 Mstone-5
Status: Available
It looks like this bug was dropped on the floor.  bryner@ just pinged me about it, but 
I'm not sure how big of an issue this is.

Comment 17 by oritm@chromium.org, Dec 17 2009

Labels: -Area-BrowserBackend Area-Internals
Replacing labels:
   Area-BrowserBackend by Area-Internals

Jay, Eric, can you please take a look at this one.
Status: Assigned
I don't think Jay is the right person here.  Eric has most recently been touching the 
protocol code and Scott's been touching the db code.  I'm assuming that this is closer 
to the protocol, so giving to eroman.
Eric -- pipe up if I'm stealing a bug from you that you were working on.
Otherwise, I think this might be a great bug for Inferno to look at. 
Status: Started
working on this.
after analyzing this and reading safe browsing doc, initial list of things to do in
order(process GURL in SafeBrowsingService::CheckUrl(const GURL& url, Client* client))

1. Remove fragment in URL.
2. Do URL unescaping until no more hex encoded characters exist.
3. In hostname, remove all leading and trailing dots.
4. In hostname, replace consecutive dots with a single dot.
5. In path, replace runs of consecutive slashes with a single slash character.
6. After performing all above steps, percent-escape all chars in url which are <=
ASCII 32, >= 127, #, %. Escapes must be uppercase hex characters.

Following canonicalization steps are not needed, as GURL already comes processed with
these.

1. Remove any tab (0x09), CR (0x0d), and LF (0x0a) chars from url. (Exclude escaped
version of these chars).
2. Normalize hostname to 4 dot-seperated decimal values.
3. Lowercase hostname.
4. Resolve path sequences of "/../" and "/./"

And finally write a unittest based on all unittests given in Section 6.1 -
http://code.google.com/p/google-safe-browsing/wiki/Protocolv2Spec

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=43100 

------------------------------------------------------------------------
r43100 | inferno@chromium.org | 2010-03-30 10:40:00 -0700 (Tue, 30 Mar 2010) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/safe_browsing/safe_browsing_util.cc?r1=43100&r2=43099
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/safe_browsing/safe_browsing_util.h?r1=43100&r2=43099
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/safe_browsing/safe_browsing_util_unittest.cc?r1=43100&r2=43099

Canonicalize the url based on Section 6.1 Safe Browsing Spec.

BUG= 7713 
TEST=SafeBrowsingUtilTest.CanonicalizeUrl
Review URL: http://codereview.chromium.org/1275002
------------------------------------------------------------------------

Status: FixUnreleased
Labels: SecSeverity-Medium
Labels: -Restrict-View-SecurityTeam
Status: Fixed
Releasing: fixed in 5.0.375.55
Labels: Type-Security
Labels: SecImpacts-Stable
Batch update.
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Mstone-5 -Area-Internals -SecSeverity-Medium -Type-Security -SecImpacts-Stable M-5 Security-Severity-Medium Cr-Internals Security-Impact-Stable Type-Bug-Security
Project Member

Comment 32 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 34 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-Medium Security_Severity-Medium
Project Member

Comment 35 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 36 by sheriffbot@chromium.org, Oct 1 2016

Labels: Restrict-View-SecurityNotify
Project Member

Comment 37 by sheriffbot@chromium.org, Oct 2 2016

Labels: -Restrict-View-SecurityNotify
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: allpublic
Project Member

Comment 39 by sheriffbot@chromium.org, Jul 29

Labels: -Pri-2 Pri-1

Sign in to add a comment