From 1ffccfb19f4b4f788597b494d99d97803dae9931 Mon Sep 17 00:00:00 2001 From: Alex Atwater Date: Sat, 28 Mar 2020 11:59:53 -0400 Subject: [PATCH 1/4] Set curated to true. --- cves/CVE-2014-0472.yml | 2 +- cves/CVE-2016-9013.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cves/CVE-2014-0472.yml b/cves/CVE-2014-0472.yml index 34a4f62..5627832 100644 --- a/cves/CVE-2014-0472.yml +++ b/cves/CVE-2014-0472.yml @@ -15,7 +15,7 @@ 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 diff --git a/cves/CVE-2016-9013.yml b/cves/CVE-2016-9013.yml index 21e74c8..9216387 100644 --- a/cves/CVE-2016-9013.yml +++ b/cves/CVE-2016-9013.yml @@ -15,7 +15,7 @@ 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 From f3d93f5c4af9caab928befbca6bd1ad889b26f5c Mon Sep 17 00:00:00 2001 From: Alex Atwater Date: Sat, 28 Mar 2020 14:39:30 -0400 Subject: [PATCH 2/4] Finish CVE-2014-0472. --- cves/CVE-2014-0472.yml | 188 +++++++++++++++++++++++++++++++---------- 1 file changed, 142 insertions(+), 46 deletions(-) diff --git a/cves/CVE-2014-0472.yml b/cves/CVE-2014-0472.yml index 5627832..449056f 100644 --- a/cves/CVE-2014-0472.yml +++ b/cves/CVE-2014-0472.yml @@ -21,18 +21,18 @@ reported_instructions: | 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 source for this is Chrome's Stable Release Channel (https://chromereleases.googleblog.com/). Please enter your date in YYYY-MM-DD format. -announced_date: +announced_date: 2014-04-21 published_instructions: | Is there a published fix or patch date for this vulnerability? Please enter your date in YYYY-MM-DD format. -published_date: +published_date: 2014-04-20 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. @@ -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 @@ -66,20 +74,31 @@ fixes_vcc_instructions: | 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. + 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: 896e3c69c7eec311085da349a329ee80c8fca132 note: This VCC was discovered automatically via archeogit. - commit: a63a83e5d88cd1696d1c40e89f254f69116c6800 - 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 I think it was a byproduct of the refactor. - commit: 1ca6e9b9e24240033349c93b05902c79c0a25bbb note: This VCC was discovered automatically via archeogit. - commit: 53e3f76d6ec5c450ab2d711368e0dd72bc39dfec @@ -97,21 +116,39 @@ vccs: - commit: a55598cbdd5e8ab58bb58e2178ccbe155526d735 note: This VCC was discovered automatically via archeogit. - commit: d7e81275242a8439768367059701baa00a9be996 - 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: 8c0eefd066aa0e5bfe8c1006d055be8e2ad69a2b note: This VCC was discovered automatically via archeogit. - commit: e0fb90b2f3b16300426851790b0ef3e50f3d67b3 note: This VCC was discovered automatically via archeogit. - commit: c01098e9cbd572eb2af97b938a7149e187561009 - 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: faf570df187bc82a3bcffa5613165a6e6cc56a57 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. More tests that + aren't relevant. - commit: b4cdf4d111e2a536abddeb38d029e71bb0d41ddb - 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: e94f405d9499d310ef58b7409a98759a5f5512b0 - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. More irrelevant + tests. - commit: 5f5f1d913bbe25dd9a33a2759144160e1473c12a - note: This VCC was discovered automatically via archeogit. + note: | + This VCC was discovered automatically via archeogit. This was where + I originally thought the problem was introduced, but this was a refactor + of moving where the function lived. - commit: 5d568bcfa66916e3de61e0090c724c899debd981 note: This VCC was discovered automatically via archeogit. - commit: 5c1143910e071c73671424036408c4548742d24f @@ -141,10 +178,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 +202,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 I am not sure 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 +227,13 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + With limited knowledge of automated tools, I don't think so. 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 +245,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 +266,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 +279,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 +303,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 needs 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 +316,9 @@ 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 +335,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 +351,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 +386,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,10 +409,14 @@ 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: | + This is the closest one that I 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. 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 From fdd216d48aa714a2057ddf6db4230de67f5fb35f Mon Sep 17 00:00:00 2001 From: Alex Atwater Date: Sun, 29 Mar 2020 12:06:11 -0400 Subject: [PATCH 3/4] Finish CVE-2016-9013 --- cves/CVE-2014-0472.yml | 10 +-- cves/CVE-2016-9013.yml | 162 ++++++++++++++++++++++++++++++----------- 2 files changed, 126 insertions(+), 46 deletions(-) diff --git a/cves/CVE-2014-0472.yml b/cves/CVE-2014-0472.yml index 449056f..9866eb9 100644 --- a/cves/CVE-2014-0472.yml +++ b/cves/CVE-2014-0472.yml @@ -68,7 +68,7 @@ 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. @@ -305,7 +305,7 @@ i18n: you did. answer: false note: | - While urls needs to be i18n compliant, this was an issue regardless of the + 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: @@ -317,8 +317,7 @@ ipc: Answer should be boolean. answer: false - note: | - Does not apply. + note: Does not apply. lessons: question: | Are there any common lessons we have learned from class that apply to this @@ -414,7 +413,8 @@ CWE_note: | This is the closest one that I 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. + 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. diff --git a/cves/CVE-2016-9013.yml b/cves/CVE-2016-9013.yml index 9216387..22e3608 100644 --- a/cves/CVE-2016-9013.yml +++ b/cves/CVE-2016-9013.yml @@ -21,18 +21,18 @@ reported_instructions: | 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 source for this is Chrome's Stable Release Channel (https://chromereleases.googleblog.com/). Please enter your date in YYYY-MM-DD format. -announced_date: +announced_date: 2016-11-01 published_instructions: | Is there a published fix or patch date for this vulnerability? Please enter your date in YYYY-MM-DD format. -published_date: +published_date: 2016-10-24 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. @@ -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,50 @@ 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. + note: This VCC was discovered automatically via archeogit. Removing imports, + nothing notable here. - 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. + 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: b8e49d70f2bbbb9008dbbf9d8b0dee46dcf25fa6 note: This VCC was discovered automatically via archeogit. - commit: ac64e91a0cadc57f4bc5cd5d66955832320ca7a1 - 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: 9dc4ba875f21d5690f6ad5995123a67a3c44bafe note: This VCC was discovered automatically via archeogit. - 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. @@ -111,10 +133,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 +155,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 I am not sure 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 +179,15 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. - answer_note: - answer: + answer_note: | + I think 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 +199,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 +223,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: | + I found 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 +237,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 +259,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 +271,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 +289,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: | + I would say 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 +339,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: | + I believe 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,10 +363,14 @@ 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 From 025dc1916b380e23fea192d3efe83ad254ff6078 Mon Sep 17 00:00:00 2001 From: Alex Atwater Date: Fri, 3 Apr 2020 14:27:00 -0400 Subject: [PATCH 4/4] Update based on feedback and add upvotes. --- Gemfile.lock | 1 + cves/CVE-2014-0472.yml | 52 ++++++------------------------------------ cves/CVE-2016-9013.yml | 23 +++++-------------- 3 files changed, 14 insertions(+), 62 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4d4b3a4..8f140eb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -22,6 +22,7 @@ GEM rspec-support (3.9.2) PLATFORMS + ruby x64-mingw32 DEPENDENCIES diff --git a/cves/CVE-2014-0472.yml b/cves/CVE-2014-0472.yml index 9866eb9..58682e6 100644 --- a/cves/CVE-2014-0472.yml +++ b/cves/CVE-2014-0472.yml @@ -89,8 +89,6 @@ vccs: 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: 896e3c69c7eec311085da349a329ee80c8fca132 - note: This VCC was discovered automatically via archeogit. - commit: a63a83e5d88cd1696d1c40e89f254f69116c6800 note: | This VCC was discovered automatically via archeogit. This one may have @@ -98,31 +96,11 @@ vccs: 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 I think it was a byproduct of the refactor. -- 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. + multiple places, but it appears to be a byproduct of the refactor. - commit: d7e81275242a8439768367059701baa00a9be996 note: | This VCC was discovered automatically via archeogit. Tests on the url reverse module but not related to issue. Next one as well. -- commit: 8c0eefd066aa0e5bfe8c1006d055be8e2ad69a2b - note: This VCC was discovered automatically via archeogit. -- commit: e0fb90b2f3b16300426851790b0ef3e50f3d67b3 - note: This VCC was discovered automatically via archeogit. - commit: c01098e9cbd572eb2af97b938a7149e187561009 note: | This VCC was discovered automatically via archeogit. This one did @@ -131,32 +109,16 @@ vccs: 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: faf570df187bc82a3bcffa5613165a6e6cc56a57 - note: | - This VCC was discovered automatically via archeogit. More tests that - aren't relevant. - commit: b4cdf4d111e2a536abddeb38d029e71bb0d41ddb 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: e94f405d9499d310ef58b7409a98759a5f5512b0 - note: | - This VCC was discovered automatically via archeogit. More irrelevant - tests. - commit: 5f5f1d913bbe25dd9a33a2759144160e1473c12a note: | - This VCC was discovered automatically via archeogit. This was where - I originally thought the problem was introduced, but this was a refactor + 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. -- 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. upvotes_instructions: | For the first round, ignore this upvotes number. @@ -164,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? @@ -203,7 +165,7 @@ discovered: If there is no evidence as to how this vulnerability was found, then please explain where you looked. answer: | - While I am not sure exactly how they discovered it (by accident or on + 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 @@ -228,7 +190,7 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. answer_note: | - With limited knowledge of automated tools, I don't think so. This would + 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 @@ -410,7 +372,7 @@ CWE_instructions: | apply here, then choose the best one and mention the others in CWE_note. CWE: 94 CWE_note: | - This is the closest one that I found after searching online. It's + 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, diff --git a/cves/CVE-2016-9013.yml b/cves/CVE-2016-9013.yml index 22e3608..b1446c7 100644 --- a/cves/CVE-2016-9013.yml +++ b/cves/CVE-2016-9013.yml @@ -80,27 +80,16 @@ fixes: - commit: 70f99952965a430daf69eeb9947079aae535d2d0 note: Cherry pick for 1.8.x branch vccs: -- commit: d595b61acae91e6111ec26cb0a4a1b1a9f4eb0d5 - note: This VCC was discovered automatically via archeogit. Removing imports, - nothing notable here. -- 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. 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: b8e49d70f2bbbb9008dbbf9d8b0dee46dcf25fa6 - note: This VCC was discovered automatically via archeogit. - commit: ac64e91a0cadc57f4bc5cd5d66955832320ca7a1 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: 9dc4ba875f21d5690f6ad5995123a67a3c44bafe - note: This VCC was discovered automatically via archeogit. - commit: 41afae4ce906838fc87d63962104cfb47991f68b note: | This VCC was discovered automatically via archeogit. Refactoring the test @@ -119,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? @@ -156,7 +145,7 @@ discovered: If there is no evidence as to how this vulnerability was found, then please explain where you looked. answer: | - While I am not sure exactly how they discovered it (by accident or on + 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 @@ -180,7 +169,7 @@ autodiscoverable: The answer field should be boolean. In answer_note, please explain why you come to that conclusion. answer_note: | - I think overall no, it's not possible since this was for a test suite + 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 @@ -224,7 +213,7 @@ subsystem: e.g. clipboard, model, view, controller, mod_dav, ui, authentication answer: | - I found a similar problem where the Oracle database had an open bug + 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. @@ -291,7 +280,7 @@ lessons: defense_in_depth: applies: true note: | - I would say having a randomized password that can't be looked up online + 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 @@ -340,7 +329,7 @@ mistakes: every one. Write a thoughtful entry here that those ing the software engineering industry would find interesting. answer: | - I believe this would be primarily a design mistake as it wasn't a fumble + 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