diff --git a/cves/CVE-2014-0472.yml b/cves/CVE-2014-0472.yml index 365edf1..3024c85 100644 --- a/cves/CVE-2014-0472.yml +++ b/cves/CVE-2014-0472.yml @@ -15,13 +15,13 @@ curated_instructions: | 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. -curated: false +curated: true reported_instructions: | What date was the vulnerability reported to the security team? Look at the security bulletins and bug reports. It is not necessarily the same day that the CVE was created. Leave blank if no date is given. Please enter your date in YYYY-MM-DD format. -reported_date: +reported_date: 2013-12-19 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 @@ -49,7 +49,15 @@ description_instructions: | Your target audience is people just like you before you took any course in security -description: +description: | + Django is a web framework that allows you to create Python classes/functions + to process certain urls. To convert between the string of the url the user + types in on their browser to the function/class that should handle the + request, the Django urlresolver's reverse function needs to lookup the + correct function/class. If A malicious hacker was able to insert malicious + code into the application, they could use this inherent trust of the system + to then look up the malicious code and run it using "dotted Python path", + or in other words, path traversal. 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 @@ -60,66 +68,57 @@ bounty: url: reviews: [] bugs: [] -repo: +repo: https://github.com/django/django 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. fixes: - commit: c1a8c420fe4b27fb2caf5e46d23b5712fc0ac535 - note: + note: This is a cherry-pick of the fix for Django 1.4.x - commit: 2a5bcb69f42b84464b24b5c835dca6467b6aa7f1 - note: + note: This is a cherry-pick of the fix for Django 1.5.x - commit: 4352a50871e239ebcdf64eee6f0b88e714015c1b - note: + note: This is a cherry-pick of the fix for Django 1.6.x - commit: 546740544d7f69254a67b06a3fc7fa0c43512958 - note: + note: This is a cherry-pick of the fix for Django 1.7.x vccs: - commit: 8d48eaa064c88533be5082e3f45638fbd48491d8 - note: This VCC was discovered automatically via archeogit. -- commit: 896e3c69c7eec311085da349a329ee80c8fca132 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This one is laying + the groundwork that will be part of the fix. It is starting to 'populate' the + available url patterns that can be used in the app as defined by the + developer, but they don't actually use them when looking up the + view to render dynamically. - commit: a63a83e5d88cd1696d1c40e89f254f69116c6800 - note: This VCC was discovered automatically via archeogit. -- commit: 1ca6e9b9e24240033349c93b05902c79c0a25bbb - note: This VCC was discovered automatically via archeogit. -- commit: 53e3f76d6ec5c450ab2d711368e0dd72bc39dfec - note: This VCC was discovered automatically via archeogit. -- commit: fb729cf1d999423721342c3dc6dfb58d48ed5389 - note: This VCC was discovered automatically via archeogit. -- commit: 37ee86b7ee63e057db0a5b868be7f799d8a4d40e - note: This VCC was discovered automatically via archeogit. -- commit: fe96214939b76d1e7389b1c569d833e96e3a5074 - note: This VCC was discovered automatically via archeogit. -- commit: 3e66913f967e01295d86a86b981e3f54fbda29a5 - note: This VCC was discovered automatically via archeogit. -- commit: 1c5fe467bd838c8521ca7c7427d8196f08a24f80 - note: This VCC was discovered automatically via archeogit. -- commit: a55598cbdd5e8ab58bb58e2178ccbe155526d735 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This one may have + some interesting aspects to look into that are beyond my knowledge of the + project. It's noted that this was a rewrite of the reverse URL parsing, + which states it fixes a lot of little bugs. It removes the line that caused + the problem in this CVE, which is good so they didn't have to fix it in + multiple places, but it appears to be a byproduct of the refactor. - commit: d7e81275242a8439768367059701baa00a9be996 - note: This VCC was discovered automatically via archeogit. -- commit: 8c0eefd066aa0e5bfe8c1006d055be8e2ad69a2b - note: This VCC was discovered automatically via archeogit. -- commit: e0fb90b2f3b16300426851790b0ef3e50f3d67b3 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. Tests on the + url reverse module but not related to issue. Next one as well. - commit: c01098e9cbd572eb2af97b938a7149e187561009 - note: This VCC was discovered automatically via archeogit. -- commit: faf570df187bc82a3bcffa5613165a6e6cc56a57 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This one did + some work on _get_url_patterns. This might have been a good red flag as to + why they weren't checking this function for when the user passed in a + URL they wanted to visit. While this is getting the valid URL's instead of + the valid views, it was a very subtle hint that they were checking some + developer-defined parts of the application. - commit: b4cdf4d111e2a536abddeb38d029e71bb0d41ddb - note: This VCC was discovered automatically via archeogit. -- commit: e94f405d9499d310ef58b7409a98759a5f5512b0 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This is doing + work around the problem area if the returned view is callable or not, but + not that the returned view is legit or not. - commit: 5f5f1d913bbe25dd9a33a2759144160e1473c12a - note: This VCC was discovered automatically via archeogit. -- commit: 5d568bcfa66916e3de61e0090c724c899debd981 - note: This VCC was discovered automatically via archeogit. -- commit: 5c1143910e071c73671424036408c4548742d24f - note: This VCC was discovered automatically via archeogit. -- commit: cfcf4b3605f9653e4e056088d89932b2a0e4281b - note: This VCC was discovered automatically via archeogit. -- commit: c3791463a5a9674f8e0148fbab57eae23c138896 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This is first + location it may appear the problem happened, but this was a refactor + of moving where the function lived. upvotes_instructions: | For the first round, ignore this upvotes number. @@ -127,7 +126,7 @@ upvotes_instructions: | 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. -upvotes: +upvotes: 1 unit_tested: question: | Were automated unit tests involved in this vulnerability? @@ -141,10 +140,17 @@ unit_tested: 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. - code: - code_answer: - fix: - fix_answer: + code: true + code_answer: | + Yes this part of the code was unit tested. However, this 'edge' case wasn't + tested since they failed to see this as a potential attack vector and + therefore didn't test for it. However, there was still able testing for + this module and subsystems. + fix: true + fix_answer: | + They updated their tests to account for this attack vector, ensuring + that view that weren't predefined by the developer were not executed + at runtime to prevent arbitrary code execution. discovered: question: | How was this vulnerability discovered? @@ -158,10 +164,17 @@ discovered: The automated, contest, and developer flags can be true, false, or nil. If there is no evidence as to how this vulnerability was found, then please explain where you looked. - answer: - automated: - contest: - developer: + answer: | + While it's unclear exactly how they discovered it (by accident or on + purpose), it does appear to come from a single developer without indication + of being automated or part of a contest. Developer was Benjamin Bach who + gave initial report and started a patch, as he is called out + in the blog post https://www.djangoproject.com/weblog/2014/apr/21/security/ + and git message on github + https://github.com/django/django/commit/8b93b31487d6d3b0fcbbd0498991ea0db9088054#diff-9175699ac06dc9232104ec6e5ed9ce42R359 + automated: false + contest: false + developer: true autodiscoverable: instructions: | Is it plausible that a fully automated tool could have discovered @@ -176,8 +189,13 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + It doesn't appear this could be autodiscoverable. This would + require simulating false code within the project (as shown by unit tests) + and adequate testing on the inside. Additionally, it wasn't a coding + mistake, but more of a high-level, domain specific, abstraction type of + attack that required some thinking to be involved. + answer: false specification: instructions: | Is there mention of a violation of a specification? For example, @@ -189,8 +207,9 @@ specification: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + No indication of specification violation. + answer: false subsystem: question: | What subsystems was the mistake in? @@ -209,8 +228,11 @@ subsystem: In the name field, a subsystem name (or an array of names) e.g. clipboard, model, view, controller, mod_dav, ui, authentication - answer: - name: + answer: | + Found in a similar bug that happened because of this bug at + https://code.djangoproject.com/ticket/22486, they were apart of the + Core (URL's) component. + name: Core URLs interesting_commits: question: | Are there any interesting commits between your VCC(s) and fix(es)? @@ -219,10 +241,21 @@ interesting_commits: interesting in light of the lessons learned from this vulnerability. Any emerging themes? commits: - - commit: - note: - - commit: - note: + - commit: 8d48eaa064c88533be5082e3f45638fbd48491d8 + note: | + This one is starting to lay down the idea of checking within the application + for items that may be okay to reference as part of dynamically looking up + URL's. In other words, they had the idea in their head when developing that + while dealing with URL's, they should look at what was already defined the + application statically before assuming they are okay at runtime. This would + have been helpful for this type of issue. + - commit: b4cdf4d111e2a536abddeb38d029e71bb0d41ddb + note: | + This is doing work around the bad line of code to check if the view that + they found for this URL is callable. They were in the right mindset of doing + validation checks on what they found (ex. that it's callable), but + they still failed to check the integrity of that view that they found + and if it's authorized to be running in the first place. i18n: question: | Was the feature impacted by this vulnerability about internationalization @@ -232,8 +265,11 @@ i18n: Answer should be boolean. Write a note about how you came to the conclusions you did. - answer: - note: + answer: false + note: | + While urls need to be i18n compliant, this was an issue regardless of the + local as it still had to check the application predefined views to see + if the found view was authorized to be called or not. ipc: question: | Did the feature that this vulnerability affected use inter-process @@ -242,8 +278,8 @@ ipc: software system reads is another form of IPC. Answer should be boolean. - answer: - note: + answer: false + note: Does not apply. lessons: question: | Are there any common lessons we have learned from class that apply to this @@ -260,8 +296,12 @@ lessons: 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. defense_in_depth: - applies: - note: + applies: true + note: | + The system should be secure so that someone can't put arbitrary code + in the system to begin with. However, it helps that they would need + to hardcode their bad code to be accepted into the application then + restart the python runtime to pickup the new config changes. least_privilege: applies: note: @@ -272,8 +312,11 @@ lessons: applies: note: distrust_input: - applies: - note: + applies: true + note: | + Usually URL's lead to other valid URL's. If a malicious actor leads + to a malicious URL that leads to arbitrary code, we need to do more + validation on what is inputted. security_by_obscurity: applies: note: @@ -304,7 +347,17 @@ mistakes: 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. - answer: + answer: | + The biggest mistake was inherent trust of the system within any part of the + server. So any runnable view that the Django application could access + on the server could be run, whether intentionally or on accident. By + using lessons that we learned in class about limiting the sandbox of + the application and ensuring that you only run code within the bounds + of the application path, it limits the attack vector by only running + code that is within the code sandbox and therefore what the Developer + intended to have run. It's more a design mistake than anything since + it almost would never make sense to want to run Django view code + outside the context of the application folder. CWE_instructions: | Please go to http://cwe.mitre.org and find the most specific, appropriate CWE entry that describes your vulnerability. We recommend going to @@ -317,12 +370,18 @@ CWE_instructions: | Just the number here is fine. No need for name or CWE prefix. If more than one apply here, then choose the best one and mention the others in CWE_note. -CWE: -CWE_note: +CWE: 94 +CWE_note: | + Closest one found after searching online. It's + not necessarily code injection as the input doesn't contain the + code, but it points to arbitrary code thta the developer didn't intend to + have run. Additionally, + https://nvd.nist.gov/vuln/detail/CVE-2014-0472 said it was CWE-94. nickname_instructions: | A catchy name for this vulnerability that would draw attention it. If the report mentions a nickname, use that. Must be under 30 characters. Optional. -nickname: +nickname: Arbitrary View Execution CVSS: AV:N/AC:H/Au:N/C:P/I:P/A:P + diff --git a/cves/CVE-2016-9013.yml b/cves/CVE-2016-9013.yml index 0a3abb0..5f198c9 100644 --- a/cves/CVE-2016-9013.yml +++ b/cves/CVE-2016-9013.yml @@ -15,13 +15,13 @@ curated_instructions: | 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. -curated: false +curated: true reported_instructions: | What date was the vulnerability reported to the security team? Look at the security bulletins and bug reports. It is not necessarily the same day that the CVE was created. Leave blank if no date is given. Please enter your date in YYYY-MM-DD format. -reported_date: +reported_date: 2016-10-25 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 @@ -49,7 +49,15 @@ description_instructions: | Your target audience is people just like you before you took any course in security -description: +description: | + Django has the built in ability to generate a TEST database to be able to + create test tables and accounts to run unit tests againt so that it + doesn't mess up the actual live database you are using. While you were + able to specify a password, if you didn't, it would use a hardcoded one + instead. Therefore, if you ended up passing the `keepdb` flag when testing + anyone who looked on the repo and found that hardcoded password + could then gain access to your database server using the default + credentials. This was only applicable when using an Oracle database. 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 @@ -60,36 +68,39 @@ bounty: url: reviews: [] bugs: [] -repo: +repo: https://github.com/django/django 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. fixes: - commit: 34e10720d81b8d407aa14d763b6a7fe8f13b4f2e - note: + note: Cherry pick for 1.10.x branch - commit: 4844d86c7728c1a5a3bbce4ad336a8d32304072b - note: + note: Cherry pick for 1.9.x branch - commit: 70f99952965a430daf69eeb9947079aae535d2d0 - note: + note: Cherry pick for 1.8.x branch vccs: -- commit: d595b61acae91e6111ec26cb0a4a1b1a9f4eb0d5 - note: This VCC was discovered automatically via archeogit. -- commit: 28308078f397d1de36fd0da417ac7da2544ba12d - note: This VCC was discovered automatically via archeogit. -- commit: d128eac316dd5a8578fbae506028a3f2ade49420 - note: This VCC was discovered automatically via archeogit. - commit: abd7e48af78bbe6ec31359c7826757d7166e55d6 - note: This VCC was discovered automatically via archeogit. -- commit: b8e49d70f2bbbb9008dbbf9d8b0dee46dcf25fa6 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This one is + doing some changes in terms of how it connects and switches it database + cursor between the main database and the test database. - commit: ac64e91a0cadc57f4bc5cd5d66955832320ca7a1 - note: This VCC was discovered automatically via archeogit. -- commit: 9dc4ba875f21d5690f6ad5995123a67a3c44bafe - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This merges in a branch + and tests for Oracle database which includes the hardcoded database + password in question. - commit: 41afae4ce906838fc87d63962104cfb47991f68b - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. Refactoring the test + database settings. This also includes how the test database properties + are stored and prefixed. - commit: ff60c5f9de3e8690d1e86f3e9e3f7248a15397c8 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This was a large + feature patch to bring in multiple database support including Oracle + database which did include refactoring and moving around + the password that was used/created in the test database. upvotes_instructions: | For the first round, ignore this upvotes number. @@ -97,7 +108,7 @@ upvotes_instructions: | 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. -upvotes: +upvotes: 1 unit_tested: question: | Were automated unit tests involved in this vulnerability? @@ -111,10 +122,15 @@ unit_tested: 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. - code: - code_answer: - fix: - fix_answer: + code: false + code_answer: | + There were no tests for this previously since there would be no point of + testing a hardcoded value against another hardcoded value. + fix: false + fix_answer: | + There were no tests added since it's a very discrete task of making it a + hardcoded random value. Therefore, there is no point if checking the + random value agains the random value as long as it's random. discovered: question: | How was this vulnerability discovered? @@ -128,10 +144,16 @@ discovered: The automated, contest, and developer flags can be true, false, or nil. If there is no evidence as to how this vulnerability was found, then please explain where you looked. - answer: - automated: - contest: - developer: + answer: | + While it's unclear exactly how they discovered it (by accident or on + purpose), it does appear to come from a single developer without indication + of being automated or part of a contest. Developer was Marti Raudsepp who + gave initial report, as he is called out + in the blog post at + https://www.djangoproject.com/weblog/2016/nov/01/security-releases/ + automated: false + contest: false + developer: true autodiscoverable: instructions: | Is it plausible that a fully automated tool could have discovered @@ -146,8 +168,15 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + Overall no, it's not possible since this was for a test suite + on a database that may optionally be kept around. There are appropriate + times, especially when testing, where you can hardcode a password. However, + this was for a specific case of a password on a specific database + where there was an option to keep the database around. An automated tool + could look for hardcoded constants of passwords, but would likely have + a lot of false positives. + answer: false specification: instructions: | Is there mention of a violation of a specification? For example, @@ -159,8 +188,12 @@ specification: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + There is no indication that there was a violation of a specification in the + blog post or the commit message. Furthermore, since this is usually fine + to hardcode a password for a test database, it would have been fine if + there wasn't an option to keep it around after testing. + answer: false subsystem: question: | What subsystems was the mistake in? @@ -179,8 +212,12 @@ subsystem: In the name field, a subsystem name (or an array of names) e.g. clipboard, model, view, controller, mod_dav, ui, authentication - answer: - name: + answer: | + There exists a similar problem where the Oracle database had an open bug + with user creation no a test database at + https://code.djangoproject.com/ticket/31153 which had Database layer + as the component. + name: Database layer interesting_commits: question: | Are there any interesting commits between your VCC(s) and fix(es)? @@ -189,10 +226,19 @@ interesting_commits: interesting in light of the lessons learned from this vulnerability. Any emerging themes? commits: - - commit: - note: - - commit: - note: + - commit: ac64e91a0cadc57f4bc5cd5d66955832320ca7a1 + note: | + This is the first merge that brings in the Oracle database work with + the hardcoded password. This was probably a first red flag that there is + a hardcoded password somewhere, but they failed to put it together + that there was also a feature that allowed you to keep around the + database if you wanted. + - commit: ff60c5f9de3e8690d1e86f3e9e3f7248a15397c8 + note: | + This VCC was discovered automatically via archeogit. This was a large + feature patch to bring in multiple database support including Oracle + database which did include refactoring and moving around + the password that was used/created in the test database. i18n: question: | Was the feature impacted by this vulnerability about internationalization @@ -202,8 +248,10 @@ i18n: Answer should be boolean. Write a note about how you came to the conclusions you did. - answer: - note: + answer: false + note: | + While passwords need to be i18n compliant, this was an issue regardless of + the local as it still had to be a random string for the password. ipc: question: | Did the feature that this vulnerability affected use inter-process @@ -212,8 +260,8 @@ ipc: software system reads is another form of IPC. Answer should be boolean. - answer: - note: + answer: false + note: Does not apply. lessons: question: | Are there any common lessons we have learned from class that apply to this @@ -230,8 +278,14 @@ lessons: 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. defense_in_depth: - applies: - note: + applies: true + note: | + Having a randomized password that can't be looked up online + would be an extra layer of complexity a malicious hacker would have to + deal with. They first need to find a Django website that uses an Oracle + database that has opted to keep around their test database AND + didn't supply their own password. Adding this is an additional layer + of security. least_privilege: applies: note: @@ -274,7 +328,18 @@ mistakes: 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. - answer: + answer: | + This would be primarily a design mistake as it wasn't a fumble + in that actual coding and execution of writing code, but rather a + collison of "features" the developers thought were in the best intention + of it's users that they failed to see how they would work when used + together. It's often helpful to keep around a test database for analysis + and it's also nice to not have to enforce a password be inputted by the + developer to get the tests running. However, the lesson to be learned here + would be when you're adding a new feature, especially a security based on, + look through the exisiting features of that component and how the new + feature would mesh and/or create unforessen complications with exisiting + features. CWE_instructions: | Please go to http://cwe.mitre.org and find the most specific, appropriate CWE entry that describes your vulnerability. We recommend going to @@ -287,12 +352,17 @@ CWE_instructions: | Just the number here is fine. No need for name or CWE prefix. If more than one apply here, then choose the best one and mention the others in CWE_note. -CWE: -CWE_note: +CWE: 798 +CWE_note: | + Since this had to do with a hard coded password, this is the closest + and very accurate CWE that notes the use of hard-coded credentials + in an application. It is also noted here: + https://nvd.nist.gov/vuln/detail/CVE-2016-9013 nickname_instructions: | A catchy name for this vulnerability that would draw attention it. If the report mentions a nickname, use that. Must be under 30 characters. Optional. -nickname: +nickname: Oracle Test Password CVSS: AV:N/AC:L/Au:N/C:P/I:P/A:P +