CVE-2016-1698

CVE: CVE-2016-1698
CWE:
- 200
- 20
bugs:
- 603725
repo: 
vccs:
- note: "In this commit, the createCustomType() method never does any validation\non
    the type corrected. This led to creating data types that could load\narbitrary
    modules, something that the second fix directly countered by\nadding a whitelist
    for module types (hard coded fix, but the number of\nAPIs used has stayed the
    same ever since the introduction of the js_modules\nin the code - crbug.com/222156
    to be specific) and then compared the \ncreated type to a type within the whitelist.\n"
  commit: 7c04d6b9c93908426b98dcb2d5fe4a90eebf9251
fixes:
- note: "Explicitly freeze the schema in chrome, pass safe arguments to \nGetAvailability,
    and broaden test access checks.'\n"
  commit: 585b125ef7168c104631e23ee5cad0108c838f52
- note: Prevent module system from loading arbitrary modules
  commit: 15574e83265d53f65ce653de0db34c738bdb89f9
bounty:
  date: '2016-06-01 17:16:00.000000000 -04:00'
  amount: 4000.0
  references:
  - http://chromereleases.googleblog.com/2016/06/stable-channel-update.html
lessons:
  yagni:
    note: 
    applies: 
  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: 
  complex_inputs:
    note: 
    applies: 
  distrust_input:
    note: "The whitelist is a prime example of distrusting input as it ensures that
      the\ninput is of a certain type; without the whitelist, these unhandled types
      can \nbe exploited to reload the page and access arbitrary data. Whitelists
      and \nblacklists are huge tools to help ensure that input is exactly how you
      want\nit to be.\n"
    applies: true
  least_privilege:
    note: 
    applies: false
  native_wrappers:
    note: 
    applies: false
  defense_in_depth:
    note: 
    applies: false
  secure_by_default:
    note: 
    applies: 
  environment_variables:
    note: 
    applies: 
  security_by_obscurity:
    note: 
    applies: 
  frameworks_are_optional:
    note: 
    applies: false
reviews:
- 1899973002
- 1912783002
- 1902263006
upvotes: 5
mistakes:
  answer: "There was a big design mistake in allowing the use of multiple types \nwithout
    validating what types are actually safe when initializing. \nOther parts of the
    code depend on the setup to work exactly as intended,\nand, without this validation,
    other parts of the code won't work in\nthe way that they should. While this fix
    is hard-coded, the addition\nof a whitelist is a huge step in the right direction.
    There could be\na better way to implement this whitelist rather than a hard-code
    \nwithin the type creation, however, although keeping that data\nsecure is very
    important.\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.
announced: '2016-06-01'
subsystem:
  name:
  - extensions
  - renderer
  answer: extensions
  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: '2016-04-14'
  answer: "This vulnerability was discovered by rob@robwu.nl when working on unit
    tests.\nInterestingly enough, Rob had come up with a large proof of concept that\ndemonstrated
    exactly how detrimental this vulnerability could be. During testing,\nhe noticed
    that the schema objects persisted across page loads, allowing it to \nbe modified
    when the page is reloaded, thus accessing arbitrary bindings and\ndisclosing information.
    The bug pages contains a lot of in-depth information\nabout this issue and is
    quite impressive as it has a high level of polish\nand testing done, especially
    for a non-Chrome employee.\n"
  google: false
  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: "The function that allows an extention to create a custom type,\ncreateCustomType,
  doesn't actually validate the module type. \nSince the type isn't ever validated,
  there's the risk of \nloading in arbitrary modules and disclosing sensitive \ninformation
  to the attacker because the definition of the module\nwas not handled properly.\n"
unit_tested:
  fix: true
  code: true
  answer: |
    A small amount of unit testing was added and is listed in code review
    1899973002, but there was a lot of work done by rob@robwu.nl whenever
    actually reporting the bug, even an entire proof of concept explained
    for testing!

    More tests were added for initializing variables to make sure that there
    aren't issues with that anymore.
  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: "The style for everying in the system was changed between the VCC and the
    \nfix for the VCC as well as a new API was added (mimeHandler) to handle\naccess
    of streams from RenderFrames to ensure that no outside streams\ncan access data.
    It handles the one specific instance at a time, creating\na major change in handing
    streams, removing the need for a user-exposed\nview ID and an event running in
    the background to handle it.\n"
  events:
  - date: Fri Apr 10 10:44:18 2015 -0700
    name: Style Standarization for Enums
  - date: Thu Jan 22 17:21:54 2015 -0800
    name: mimeHandler Extension API Added
  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: |
    It took a very long time to actually notice this bug - the part of the
    code that didn't validate variable types was created in 2013 but not
    noticed until 2016. The commit history is very interesting to read as
    you notice how it wasn't questioned for years.
  commits:
  - note: "There was another bug being fixed where one commit within the binding.js
      file\nbroke various builds of Chromium due to not correcting old resource references.
      \nI find it interesting that it took awhile to fix that issue when it has a
      \nhigher impact on the system overall - maybe the cases where it broke weren't\nbeing
      tested as much?\n"
    commit: b155037ce9ddda46e7260c94bd8cfccdc9fc6404
  - note: |
      This commit reverted a commit (68c06a78f74212907faafd985b0bdee67a6d4120
      to be exact) that was made 8 days prior. It caused a performance regression
      and was noted that "it should be revisited later". The commit prior resolved
      a dependency issue, but this specific commit removed that code entirely and
      wasn't reworked until a later commit. It's interesting to note how much code
      can change in a small amount of time and that the same person was able to
      realize this issue by himself.
    commit: 33076cbe96e8eb8c50541e40f5266955c3db667f
  - note: "This commit was just a style fix, but it's interesting to see how this
      \nwas only considered in 2015, 2 years after the file's creation. Coding\nstandards
      are different from person to person, and this person who \nmade the commit (rdevlin.cronin@chromium.org)
      felt that a different\nstyle was necessary. Considering it was changing enums
      to be in all\ncaps, a now commonly recognized and used styling for enums, this
      was\nan important fix that can help reduce confusion when other employees\nreview
      it or try to refactor code.\n"
    commit: d0ea23a1ad1e50dc083b68984035fea153b30eb2
  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.