CVE-2010-1505

aka Off to the Races

CVE: CVE-2010-1505
CWE:
- 362
- 264
bugs:
- 40575
- 12718
repo: 
vccs:
- note: |
    Large refactoring of RenderViewHost that included addition of logic to check if a destination URL already had a SiteInstance
      or was a restore; this is where the code was missing the check added in the fix to see if the
      URL has a DOMUIScheme.
  commit: b6e09acf8ced26198871626c76bb5a3741cc51f1
fixes:
- note: |
    Added safety check to ensure tabs can only navigate to DOM-UI-permitted URLS (actual fix in 54ec647 below, lines were
    removed in this commit because of an issue with an automated check).
  commit: 821825658763d672a40ff332753103108323dbaa
- note: "Eagerly set the site instance as soon as a new tab process begins to ensure
    that if we navigate away from that new tab, \nit has its own process and does
    not share the process of the new tab.\n"
  commit: 9821e6e9f5259141e3e6c25c3b9493c78ae08e2c
- note: 'Added safety check to ensure tabs can only navigate to DOM-UI-permitted URLS.

    '
  commit: 54ec647bc4c41a46bab353c9eede6441030be150
bounty:
  date: 
  amount: 
  references:
  - https://chromereleases.googleblog.com/2010/04/stable-update-security-fixes.html
  - https://bugs.chromium.org/p/chromium/issues/detail?id=40575#c2
lessons:
  yagni:
    note: 
    applies: false
  question: |
    Are there any common lessons we have learned from class that apply to this
    vulnerability? In other words, could this vulnerability serve as an example
    of one of those lessons?

    Leave "applies" blank or put false if you did not see that lesson (you do
    not need to put a reason). Put "true" if you feel the lesson applies and put
    a quick explanation of how it applies.

    Don't feel the need to claim that ALL of these apply, but it's pretty likely
    that one or two of them apply.

    If you think of another lesson we covered in class that applies here, feel
    free to give it a small name and add one in the same format as these.
  serial_killer:
    note: 
    applies: false
  complex_inputs:
    note: 
    applies: false
  distrust_input:
    note: 
    applies: false
  least_privilege:
    note: |
      The fix for this vulnerability did not involve changing permissions to be less
      comprehensive, but it is worth noting that possible exploits could have taken
      advantage of the fact that the newly-rendered page erroneously had the same
      permissions as the new tab process.  If the new tab process has fewer permissions,
      exploitation may have been less of a concern.
    applies: true
  native_wrappers:
    note: 
    applies: false
  defense_in_depth:
    note: "Commit #9821e6e9f5259141e3e6c25c3b9493c78ae08e2c contained the main fix
      for the\nvulnerability, but team discussion (https://codereview.chromium.org/1549027)
      \nprompted devs to add another defensive check that would crash the browser
      if \nthe vulnerability was taken advantage of again, helping them catch future
      bugs.\n"
    applies: true
  secure_by_default:
    note: 
    applies: false
  environment_variables:
    note: 
    applies: false
  security_by_obscurity:
    note: 
    applies: false
  frameworks_are_optional:
    note: 
    applies: false
reviews:
- 1626012
- 1519025
- 1549027
- 1523018
- 1631009
upvotes: 14
mistakes:
  answer: "At the VCC and between the VCC and fix, there appears to have been extensive
    refactoring\nwith moving functionality to new directories.  This was done in batches
    of massive commits, which\nlikely made it more challenging to identify issues
    and vulnerabilities in the files involved.  This\ncould be classified as a mistake
    made in version control practices.\n\nAs for the vulnerability itself, the fixes
    are simple and seem appropriate, and the developers\nhad the foresight to practice
    defense in depth by adding an additional safety check after the initial\nfix was
    made.  Overall, the only fix that needed to be made was an additional check for
    DOMUIScheme; the\nvulnerability appears to have been a result of a simple coding
    mistake.  \n\nOne could argue that a greater design mistake is implied by this
    vulnerability since it occurred\ndue to a race condition.  The condition could
    potentially lead to further vulnerabilities in the future; however,\nthe fix did
    not address this overall issue but rather the specifics of this vulnerability.\n"
  question: |
    In your opinion, after all of this research, what mistakes were made that
    led to this vulnerability? Coding mistakes? Design mistakes?
    Maintainability? Requirements? Miscommunications?

    Look at the CWE entry for this vulnerability and examine the mitigations
    they have written there. Are they doing those? Does the fix look proper?

    Use those questions to inspire your answer. Don't feel obligated to answer
    every one. Write a thoughtful entry here that those ing the software
    engineering industry would find interesting.
nickname: Off to the Races
announced: '2010-04-20'
subsystem:
  name:
  - tab_contents
  - renderer_host
  answer: 'Based on paths of affected files.

    '
  question: |
    What subsystems was the mistake in?

    Look at the path of the source code files code that were fixed to get
    directory names. Look at comments in the code. Look at the bug reports how
    the bug report was tagged. Examples: "clipboard", "gpu", "ssl", "speech", "renderer"
discovered:
  date: '2010-04-06'
  answer: |
    This vulnerability was reported by Google employee eroman@chromium.org.  The bug report
    discussion does not say how he found it, but does describe how to recreate it and notes
    that the incorrect setting of the 'dom_ui_enabled' bit can be seen by attaching to the
    renderer process hosting the test webpage under 'RenderView.' This may have been discovered
    via an existing bug report (12718) that turned out to be a side effect of the wider issue
    captured by this vulnerability.
  google: true
  contest: false
  question: |
    How was this vulnerability discovered?

    Go to the bug report and read the conversation to find out how this was
    originally found. Answer in longform below in "answer", fill in the date in
    YYYY-MM-DD, and then determine if the vulnerability was found by a Google
    employee (you can tell from their email address). If it's clear that the
    vulenrability was discovered by a contest, fill in the name there.

    The "automated" flag can be true, false, or nil.
    The "google" flag can be true, false, or nil.

    If there is no evidence as to how this vulnerability was found, then you may
    leave the entries blank except for "answer". Write down where you looked in "answer".
  automated: false
description: "When opening a new tab, one could take advantage of a race condition
  and\nstart loading a page in that tab before the new tab process (NTP) completed.
  \ \nThis resulted in the newly-loaded tab sharing the privileges of the new tab
  page.  \nThis vulnerability was the underlying cause of another bug (Chromium bug
  12718) \nwhich saw form submissions being submitted as a GET instead of a POST since
  the \npage was loaded in the new tab process.  With the privileges of the new tab
  process,\nan attacker may have been able to execute a CSRF or similar attack taking
  advantage of\nthe side effects of these privileges (such as the change of form submissions
  from POST \nto GET).\n"
unit_tested:
  fix: true
  code: false
  answer: "There did not appear to be any unit tests involved with this vulnerability.
    \ A unit test might \nhave checked the state of the site instance variable, but
    because this was originally not immediately set\nunless a new tab was a page reload,
    it was not necessary.  In most cases, the site instance was not set until the\nnew
    tab process finished, so there was no relevant state change to check.\n\nDev added
    two EXPECT statements validating the state of the site instance, as the\nprimary
    fix involved setting the site instance variable as soon as a new tab is opened,
    possibly before the new\ntab process has completed, to prevent a quick navigation
    to a new page from inheriting the new tab's site instance\nand permissions.\n"
  question: |
    Were automated unit tests involved in this vulnerability?
    Was the original code unit tested, or not unit tested? Did the fix involve
    improving the automated tests?

    For the "code" answer below, look not only at the fix but the surrounding
    code near the fix and determine if and was there were unit tests involved
    for this module.

    For the "fix" answer below, check if the fix for the vulnerability involves
    adding or improving an automated test to ensure this doesn't happen again.
major_events:
  answer: 'See below.

    '
  events:
  - date: '2009-04-22'
    name: Start of refactoring moving members of WebContents to new TabContents
  - date: '2009-05-27'
    name: Google Chrome sends GET request for POST form bug reported (12718)
  - date: '2010-03-29'
    name: Commenter on 12718 thread realizes bug may be a result of a race condition
  - date: '2010-04-07'
    name: Decided on initial fix via eagerly assigning SiteInstance
  - date: '2010-04-08'
    name: Request for follow-up change made to check for navigation to non-DOM-UI
      URL
  question: |
    Please record any major events you found in the history of this
    vulnerability. Was the code rewritten at some point? Was a nearby subsystem
    changed? Did the team change?

    The event doesn't need to be directly related to this vulnerability, rather,
    we want to capture what the development team was dealing with at the time.
curation_level: 1
CWE_instructions: |
  Please go to cwe.mitre.org and find the most specific, appropriate CWE entry
  that describes your vulnerability. (Tip: this may not be a good one to start
  with - spend time understanding this vulnerability before making your choice!)
bounty_instructions: |
  If you came across any indications that a bounty was paid out for this
  vulnerability, fill it out here. Or correct it if the information already here
  was wrong. Otherwise, leave it blank.
interesting_commits:
  answer: 'See below.

    '
  commits:
  - note: "This was a fairly large refactoring effort for the RenderViewHost, which
      was used to\nrender the new tabs in this vulnerability.  While this refactoring
      may not have\ncontributed directly to the VCC, it is very large (1645 insertions
      and 1226 deletions),\nwhich may be a red flag about the size of the commits
      developers were making in 2008.  \nIf commits of this size were commonplace,
      it would be much more difficult to locate the root\nof a vulnerability.\n\nThis
      commit was actually the VCC, but the commit message was misleading because it
      indicated \nthat it was only a refactoring.  Most of it was indeed refactoring,
      but buried within all of \nthose changes was the addition of logic to check
      if a destination URL already had a SiteInstance\nor was a restore; this is where
      the code was missing the check added in the fix to see if the\nURL has a DOMUIScheme.\n\nIt
      appears this file was not touched again until it was at some point moved into
      a tab_contents\nfolder, then later when the fix was made.\n"
    commit: b6e09acf8ced26198871626c76bb5a3741cc51f1
  - note: "This seems to have been another refactoring commit where much of the tab
      functionality was moved to \na specific directory.  While it isn't as large
      as the last commit, it is still sizeable with 447 \ninsertions and 478 deletions.\n"
    commit: 96d185dff2231e09dfcf4b08c9ed878a4a9f7acf
  - note: "This commit was the start of moving functionality from WebContents (which
      was heavily involved in \nthe VCC commit) to a separate TabContents directory.
      This reorganization may have played a part in\nidentifying the initial bug with
      form submission behavior, as that bug was recorded about a month later.\nUpon
      reading the discussions in the bug report for the form submission issue, it
      is interesting that no\none seems to have brought up this then-recent refactoring.\n"
    commit: fdd61c60136a6d3cf351003802b513a950c91029
  question: |
    Are there any interesting commits between your VCC(s) and fix(es)?

    Write a brief (under 100 words) description of why you think this commit was
    interesting in light of the lessons learned from this vulnerability. Any
    emerging themes?

    If there are no interesting commits, demonstrate that you completed this section by explaining what happened between the VCCs and the fix.
curated_instructions: |
  If you are manually editing this file, then you are "curating" it. Set the
  entry below to "true" as soon as you start. This will enable additional
  integrity checks on this file to make sure you fill everything out properly.
  If you are a student, we cannot accept your work as finished unless curated is
  set to true.
upvotes_instructions: |
  For the first round, ignore this upvotes number.

  For the second round of reviewing, you will be giving a certain amount of
  upvotes to each vulnerability you see. Your peers will tell you how
  interesting they think this vulnerability is, and you'll add that to the
  upvotes score on your branch.
announced_instructions: |
  Was there a date that this vulnerability was announced to the world? You can
  find this in changelogs, blogs, bug reports, or perhaps the CVE date. A good
  source for this is Chrome's Stable Release Channel
  (https://chromereleases.googleblog.com/).
  Please enter your date in YYYY-MM-DD format.
fixes_vcc_instructions: |
  Please put the commit hash in "commit" below (see my example in
  CVE-2011-3092.yml). Fixes and VCCs follow the same format.
description_instructions: |
  You can get an initial description from the CVE entry on cve.mitre.org. These
  descriptions are a fine start, but they can be kind of jargony.

  Rewrite this description in your own words. Make it interesting and easy to
  read to anyone with some programming experience. We can always pull up the NVD
  description later to get more technical.

  Try to still be specific in your description, but remove Chromium-specific
  stuff. Remove references to versions, specific filenames, and other jargon
  that outsiders to Chromium would not understand. Technology like "regular
  expressions" is fine, and security phrases like "invalid write" are fine to
  keep too.