New issue
Advanced search Search tips

Issue 815217 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

UnguessableToken::ToString() does not guarantee string length

Project Member Reported by eladalon@chromium.org, Feb 23 2018

Issue description

One would expect the string representation of an unguessable token to always be the same length, because it always represents 128b of randomness. However, when the most significant four bytes of either the high_ or the low_ part are zero, which happens about (1 - ((15/16)^2) of the time, the string would be shorter.
 
Labels: -Pri-2 Pri-1
Actually, this is a bit worse. Observe the following code:
  UnguessableToken token1 = UnguessableToken::Deserialize(0x0000000012345678, 0x0000000123456789);
  UnguessableToken token2 = UnguessableToken::Deserialize(0x0000000123456781, 0x0000000023456789);
  EXPECT_NE(token1.ToString(), token2.ToString());
You would expect the two string to be different, but they are, in fact, the same.
Status: Assigned (was: Untriaged)
Fix here: https://chromium-review.googlesource.com/c/chromium/src/+/934824
Labels: Restrict-View-SecurityTeam OS-Android OS-Chrome OS-Fuchsia OS-iOS OS-Linux OS-Mac OS-Windows
Cc: yucliu@chromium.org tguilbert@chromium.org pfeldman@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 26 2018

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

commit d16e74c356a770d216177213eec390790eca75dc
Author: Elad Alon <eladalon@chromium.org>
Date: Mon Feb 26 13:16:50 2018

Fix UnguessableToken::ToString()

Make sure that ToString() would produce distinct strings for
distinct tokens.

Bug:  815217 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I38ea5bfad061610e95945eb31187cb3c02f737cf
Reviewed-on: https://chromium-review.googlesource.com/934824
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Elad Alon <eladalon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539115}
[modify] https://crrev.com/d16e74c356a770d216177213eec390790eca75dc/base/unguessable_token.cc
[modify] https://crrev.com/d16e74c356a770d216177213eec390790eca75dc/base/unguessable_token_unittest.cc
[modify] https://crrev.com/d16e74c356a770d216177213eec390790eca75dc/components/viz/common/surfaces/local_surface_id_unittest.cc

Status: Fixed (was: Assigned)
Project Member

Comment 7 by sheriffbot@chromium.org, Feb 26 2018

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

Comment 8 by sheriffbot@chromium.org, Jun 4 2018

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

Sign in to add a comment