Mapping to CWE and CVSS #33

jcran · 2017-06-10T03:31:01Z

Adding a mapping for CWE and CVSS based on the work @jhaddix and team have done. Note that it looks like there may be some updates to the VRT since this work was completed (so a few left to map), and we may want to roll some of these up to the top level category vs having many variants with the same. Consider this a first pass, and in need of additional eyeballs & help!

Resolves #32

plr0man · 2017-06-14T21:22:05Z

Thanks for your great work guys! This might be a good moment to discuss if we want reference and/or how-to info to be present in the main VRT file as more new entries (some heavy) are one the way. I am not opposed to having it all in one file, but I'm curious to hear your thoughts.

Potential separation into two files:

VRT 'light' and VRT 'full' (full = light+references)
VRT 'base' and VRT 'extension' (extension = references only)

Why I am considering this solution:

Main VRT stays lightweight
Presentation/Resource type of entries/schema are separated from main application logic entries/schema
References might require quicker/different release process
Eliminating redundancy as all entries should have references, but some may share the same

jcran · 2017-06-14T22:50:10Z

more new entries (some heavy) are on the way

Can you give any more context?

ryancblack · 2017-06-19T23:31:26Z

@jcran @barnett - given the idea of adding other mappings in the future, from a design perspective, do you feel we could instead consider a mapping file that refers to the VRT ID in lieu of incorporating this into the VRT itself?

It would be beneficial to keep the VRT file fairly lightweight and focused. If we have CVSS/CWE live in a mapping file it allows this separation.

jcran · 2017-06-30T21:34:49Z

bump Does this need to be moved to a separate file?

barnett · 2017-07-08T00:18:56Z

After talking about this internally we’ve decided creating the mapping in a separate repository will be optimal. It allows us to keep the responsibilities separated and be able to define mappings to previous VRT releases, without adjusting previous version tags.

The way we see it being the easiest to work with would be a format like below:

{
  "sensitive_data_exposure.sensative_data_hardcoded.oauth_secret": {
    "cwe": ["CWE-311"],
    "cvss_base_vector": "AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:N"
  }
}

We would then have the logic check the most specific ID, then bump up a level if no match each time. So if no variant match, and a subcategory could match, we would pull that. This allows us to ease the work if the mapping is shared across all variants so that changes to those variants don’t require changes in the mapping.

One question we had, will we need to support mapping of non-current VRT versions? For example, if we had a previous VRT version, would ya need to add a new mapping to the previous version but not the latest? I assume not, but wanted to confirm.

plr0man · 2017-07-08T03:34:58Z

vulnerability-rating-taxonomy.json

+							"priority": 4,
+							"mapping": {
+								"cwe": ["AV:N/AC:H/PR:N/UI:R/S:U/C:L/I:L/A:N"],
+								"cvss_base_vector": "CWE-451"


CWE and CVSS fields mismatched

plr0man · 2017-07-08T03:59:37Z

vulnerability-rating-taxonomy.json

+							"type": "variant",
+							"priority": 4,
+							"mapping": {
+								"cwe": ["CWE-933", "CWE-524"],


CWE-524 should go first as a smaller number, this way we can ensure we are able to locate cwe fields with identical values. In this case there is cache_control_for_a_non_sensitive_page that has "cwe": ["CWE-524", "CWE-933"],

plr0man · 2017-07-08T05:05:52Z

I like the idea of mapping per subcategory level if a field is shared across all variants. I believe that CWE and CVSS mappings would have to be separated for that purpose. If you look at the variants with the same CWE you will notice that CVSS fields vary. Separating CVSS and CWE mappings would also provide the advantage of mapping multiple unrelated IDs to one CWE/CVSS field. Looks like there are 169 entries total, but only 38 unique CVSS fields and 49 unique CWE fields.
Regarding your question, let's discuss it internally, so we don't miss any context.

…cwe-and-cvss

tessereth · 2017-09-06T06:53:57Z

First of all, this looks great and thanks for all the work people have put into this. I've updated this branch to match master. I added a bunch of TODO markers for new and merged nodes. I definitely don't have the knowledge to be able to set the values, so if anyone else has time, that would be great.

As for other comments in this thread, I figured I'd include my two cents.

Personally, I think it makes more sense to include the mappings in with everything else so they get updated when nodes are added/removed/merged etc. But I don't have strong opinions on whether it should be in the same file or another one. It should be easy enough to use the data either way.
As for using a separate repository, maintaining a whole extra repo seems like making more work than we need to. If that was mainly suggested for backwards compatibility with previous versions of the VRT, I think we can handle that in code using the deprecated node mapping. I'd have to actually try it to be sure though.
Having mappings for internal nodes seems like a good idea. Easy to implement the way @barnett suggested. We can separate CWE and CVSS if that's useful, allowing something like the following. We can add validation to ensure all leaf nodes and a CWE and CVSS mapping, even if it comes from a parent

      "id": "insecure_data_storage",
      "type": "category",
      "children": [
        {
          "id": "sensitive_application_data_stored_unencrypted",
          "type": "subcategory",
          "mapping": {
            "cwe": ["CWE-312"]
          },
          "children": [
            {
              "id": "on_external_storage",
              "type": "variant",
              "mapping": {
                "cvss_base_vector": "AV:P/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:N"
              }
            },
            {
              "id": "on_internal_storage",
              "name": "On Internal Storage",
              "type": "variant",
              "priority": 5,
              "mapping": {
                "cvss_base_vector": "AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:N"
              }
            }
          ]
        }

Separating CVSS and CWE mappings would also provide the advantage of mapping multiple unrelated IDs to one CWE/CVSS field.

Hm. Reading that, it suggests you want to do something like:

{
  "cwe": {
    "CWE-123": ["vrt-id-one", "vrt-id-two"],
    ...
  },
  "cvss": {
    "AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:N": ["vrt-id-one", "vrt-id-three"],
    ...
  }
}

So that does save you the effort of repeating the CWE and CVSS scores but then you have to repeat the vrt ids. So it may not end up helping that much. I can be persuaded on that, but in particular for CVSS, going in that direction doesn't make a lot of sense to me.

adamrdavid · 2017-09-07T00:09:29Z

@tessereth I agree the separate repo would be a bit much.

I think the main arguments for having it in a different file are separation of concerns and as @barnett suggested in:

This allows us to ease the work if the mapping is shared across all variants so that changes to those variants don’t require changes in the mapping.

This will make for more concise diffs, especially considering the possibility of adding another mapping. Though we will of course need to add more validation. If it is in a separate file, I'd vote for the format to have the vrt_ids as the root key instead of:

{
  "cwe": {
    "CWE-123": ["vrt-id-one", "vrt-id-two"],
    ...
  },
  "cvss": {
    "AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:N": ["vrt-id-one", "vrt-id-three"],
    ...
  }
}

We could still do it without repetition in one file by adding some logic to look at parent nodes for a mapping, but I don't think that is even necessary (repetition in json feels fine 🤷‍♂️ ).

I could be swayed either way, so leaving that decision in your hands.

As for the questions on versioning, that can all be handled by the deprecated node logic as is. The VRT.find_node method will give you the most up to date match which will have the most up to date mapping.

plr0man · 2017-09-08T18:03:24Z

CHANGELOG.md

@@ -75,6 +75,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 - insufficient_security_configurability.weak_password_policy.allows_password_to_be_same_as_email_username

 ### Changed
+- adjusted schema and added a (currently optional) CWE and CVSS base score for most entries


Let's keep the changelog chronological, meaning appending changelog entries to the bottom of a particular section

plr0man · 2017-09-08T18:04:58Z

vrt.schema.json

+                              "type": "object",
+                              "properties": {
+                                "cwe" : {
+                                  "type" : "array",


Can we have this part in one line?

plr0man · 2017-09-08T18:05:18Z

vrt.schema.json

+                                  }
+                                },
+                                "cvss_base_vector" : {
+                                  "type": "string",


Can we have this part in one line?

plr0man · 2017-09-08T18:14:27Z

vulnerability-rating-taxonomy.json

+              "mapping": {
+                "cwe": ["AV:N/AC:H/PR:N/UI:R/S:U/C:L/I:L/A:N"],
+                "cvss_base_vector": "CWE-451"
+              }


CWE and CVSS fields mismatched

plr0man · 2017-09-08T18:15:47Z

vulnerability-rating-taxonomy.json

+              "priority": 4,
+              "mapping": {
+                "cwe": ["CWE-933", "CWE-524"],
+                "cvss_base_vector": "AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:N"


CWE-524 should go first as a smaller number, this way we can ensure we are able to locate cwe fields with identical values. In this case there is cache_control_for_a_non_sensitive_page that has "cwe": ["CWE-524", "CWE-933"],

tessereth · 2017-09-12T06:14:52Z

I created a new pull request #86 so I'm not completely breaking the branch here. If interested parties could have a look that would be great.

barnett · 2017-09-20T18:33:28Z

Closed in favor of the implementation in #86

First pass at mapping to CWE and CVSS, thanks to @jhaddix & team

636d9f0

plr0man added the enhancement label Jun 10, 2017

plr0man added this to the v2.0 milestone Jun 10, 2017

plr0man reviewed Jul 8, 2017

View reviewed changes

tessereth added 2 commits September 6, 2017 15:53

Merge remote-tracking branch 'origin/master' into 32-enhancement-add-…

951c0f7

…cwe-and-cvss

fix json validation

c162162

Merge branch 'master' into 32-enhancement-add-cwe-and-cvss

5f5db8c

barnett modified the milestones: v2.0, v1.3 Sep 7, 2017

plr0man reviewed Sep 8, 2017

View reviewed changes

tessereth mentioned this pull request Sep 12, 2017

Add CVSS mapping #86

Merged

3 tasks

barnett closed this Sep 20, 2017

barnett deleted the 32-enhancement-add-cwe-and-cvss branch September 21, 2017 20:05

jcran mentioned this pull request Oct 13, 2017

CWE mapping #99

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mapping to CWE and CVSS #33

Mapping to CWE and CVSS #33

jcran commented Jun 10, 2017 •

edited by barnett

plr0man commented Jun 14, 2017 •

edited

jcran commented Jun 14, 2017 •

edited

ryancblack commented Jun 19, 2017 •

edited

jcran commented Jun 30, 2017

barnett commented Jul 8, 2017

plr0man Jul 8, 2017

plr0man Jul 8, 2017

plr0man commented Jul 8, 2017

tessereth commented Sep 6, 2017

adamrdavid commented Sep 7, 2017

plr0man Sep 8, 2017

plr0man Sep 8, 2017

plr0man Sep 8, 2017

plr0man Sep 8, 2017

plr0man Sep 8, 2017

tessereth commented Sep 12, 2017

barnett commented Sep 20, 2017

Mapping to CWE and CVSS #33

Mapping to CWE and CVSS #33

Conversation

jcran commented Jun 10, 2017 • edited by barnett

plr0man commented Jun 14, 2017 • edited

jcran commented Jun 14, 2017 • edited

ryancblack commented Jun 19, 2017 • edited

jcran commented Jun 30, 2017

barnett commented Jul 8, 2017

plr0man Jul 8, 2017

Choose a reason for hiding this comment

plr0man Jul 8, 2017

Choose a reason for hiding this comment

plr0man commented Jul 8, 2017

tessereth commented Sep 6, 2017

adamrdavid commented Sep 7, 2017

plr0man Sep 8, 2017

Choose a reason for hiding this comment

plr0man Sep 8, 2017

Choose a reason for hiding this comment

plr0man Sep 8, 2017

Choose a reason for hiding this comment

plr0man Sep 8, 2017

Choose a reason for hiding this comment

plr0man Sep 8, 2017

Choose a reason for hiding this comment

tessereth commented Sep 12, 2017

barnett commented Sep 20, 2017

jcran commented Jun 10, 2017 •

edited by barnett

plr0man commented Jun 14, 2017 •

edited

jcran commented Jun 14, 2017 •

edited

ryancblack commented Jun 19, 2017 •

edited