Uploaded image for project: 'Minimal Marketable Features'
  1. Minimal Marketable Features
  2. MMF-1249

Security Auditors have some hints to drive their manual source code review

    Details

    • Type: MMF
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Labels:

      Description

      WHY

      With MMF-1189, SQ/SC are about to become powerful SAST solutions to detect most of the well-known injection vulnerabilities. That's great but whatever the maturity level of a SAST solution used to scan a software, a lot of vulnerabilities can only be spotted through a careful manual code review process. As a Security Auditor I would also like SQ/SC to help me drive my manual code review process by highlighting sensitive pieces of code (Security Hotspots) such as use of an encryption algorithm, execution of an OS command, access to the file system, and so on. Once these sensitive places in the code are identified, it's up to me as the auditor to determine whether a vulnerability exists at each spot.

      Indeed let's imagine that tomorrow I'm in charge of auditing an application with 200K lines of code. If SQ/SC highlights all the pieces of code which are of interest from a security perspective it can save me a lot of time to have these hotspots pinpointed, while allowing me to be even more accurate.

      As a Security Auditor:

      • I need a way to quickly get access to the main areas at risk on a software.
      • I have the skills to qualify a security flaw and when an issue type is Vulnerability it should be a no-brainer for all stakeholders that the vulnerability is a true one which can be exploited in a software execution context.

      WHAT

      Definition

      A new issue type called: "Security Hotspot" will be added to support this audit effort to allow a security auditor to quickly focus on what is sensitive, such as:

      SonarAnalyzers will generate either Vulnerabilities or Hotspots. The choice between the two types will be led by how confident we are the rule implementation raises real issues, and whether such issues must be reviewed to take into account the execution context. RSPECs will be updated accordingly once this new type is in place.

      From a technical standpoint, a "Security Hotspot" is a new type of issue. The current functionality in place to raise other issue types should largely be reused to implement the "Security Hotspot" type.

      The Security Hotspot issue type should be available to external issues (MMF-1212), however no special provisions will be made for them. Specifically, the limitations on being able to manage resolution (i.e. can't set to FP/WF) will remain in force for Hotspots, so this may ultimately be an unsatisfying experience, and the assumption is that most external hotspot issues will be imported through full plugins.

      Rules

      Hotspot rules will be visible in the Rules space and included in profiles just as any other rules are. They will also be reflected in rules.sonarsource.com.

      "Security Hotspot" lifecycle

      A code analyzer finds a Security Hotspot, let's say the use of a cookie, and raises a Hotspot issue. That issue is assigned to the developer who last touched the line (as usual) but the developer receives no notifications of this issue. This issues is (eventually - MMF-1266) not visible on the Issues page nor in SonarLint. In short, the developer has no knowledge of the issue at this stage.

      Instead, the issue goes into the Security Auditor's queue for review. If after examining the context, she determines that a co-located Vulnerability exists (sensitive information is being stored in the cookie) she will change the type of the issue to Vulnerability. Ideally, she is prompted to make a comment.

      At this point, the issue is visible on the Issues page, the normal notifications are fired, and a "Manual" label is added to the issue block (with appropriate explanatory mouseover). This label indicates to the developer that manual intervention will be required to get it back out of her queue.

      The developer removes the sensitive information from the value being stored in the cookie. The next analysis does not close the Manual Vulnerability because the use of the cookie remains. Instead, the developer moves the issue to the "To Review" resolution, which is only available for Manual Vulnerabilities. This update turns the issue back into a Hotspot, which removes the issue from the Issues page.

      The auditor must now notice that the issue has been returned to her queue (MMF-1266 will include a specific display of the count of To Review issues). No notifications are sent.

      If the Vulnerability has been fixed, the auditor will move it to the WF resolution.

      If the Vulnerability has not been fixed, the auditor will change it back into a Manual Vulnerability, thus resetting the status to Open and moving it back to the Issues page and the developer's queue.

      A diagram of the proposed lifecycle is attached:

      To be clear, this diagram includes Closed and Removed statuses because it is still possible for the use of the Cookie to be removed (Closed) or for the Hotspot rule to be removed from the quality profile (Removed).

      A diagram of the current issue lifecycle (type transitions omitted) is attached for comparison:

      Given this lifecycle, only a subset of dispositions is appropriate for Security Hotspots: Open/ReOpened, Won't Fix, Removed, Fixed and the new "To Review". Explicitly, there is no need to "resolve as fixed" or "confirm" a Hotspot; if a vulnerability is found at the site of a Hotspot, that Hotspot will be converted to a Vulnerability. Further, "False Positive" should also be unnecessary since 99.9% of the time, if we raise a Vulnerability Hotspot issue, a human really does need to look at it.

      As a security auditor, I will be able to move an issue back and forth between Hotspot and Manual Vulnerability. There should be no option to convert a Hotspot or Manual Vulnerability to any other type.
      As a security auditor and as a developer:

      • I can see the Hotspot severity on the issues page, but I can't change it.
      • I can't change the assignee on a Hotspot.

      Permissions

      A permission "Administer Security Hotspots" should be added to control who is authorized to transform a "Security Hotspot" into a Vulnerability and vice versa. This new Administer Security Hotspots permission will be in the default permissions template.

      For existing projects, users and groups having the permission "Administer Issues" will be granted the permission "Administer Security Hotspots".

      Visibility

      This new type should not be visible in the project, portfolio, or application homepage; or in SonarLint.

      Eventually, Security Hotspots will be viewed and managed in a new, dedicated space (MMF-1266). Until that space is available, they will show up in the Issues page.

      Measures

      Hotspots should not impact the Security Rating (on New Code), (New) Vulnerabilities, or Security Remediation Effort (on New Code) measures. Hotspots should also not be reflected in the raw issue count measures: (New) Issues, False Positive Issues, Open Issues, Reopened Issues, (New) Blocker Issues, ...

      Two new metrics should be added: "Security Hotspots" and "New Security Hotspots". There is no point in creating the related remediation effort metrics since Hotspots are only "potential" issues and any remediation cost associated with them is there only in case they are converted to Vulnerabilities.

      Security Hotspot issues will be counted in raw issues count measure to be consitent with what is displayed in the issues page.

      Issue Assignment and Notifications

      Hotspots will be automatically assigned at creation, but these assignments (or any subsequent assignments made via web services) should not generate or be included in notifications. A notification should be generated only if a Hotspot is converted into a Vulnerability.

      Backward Compatibility

      "Security Hotspots" should be considered as "Vulnerabilities" for the versions of SonarQube prior to the one implementing this MMF.

      Historical Recovery

      When a rule is moved from a Vulnerability to a Hotspot in RSPEC, related existing issues (created as automatic Vulnerabilities) should be migrated at next analysis into Hotspot issues. Whatever the current status of the Vulnerability, the migrated Hotspot will be in the "Open" status waiting to be reviewed by a security auditor.
      It is accepted to have a discrepancy for projects not re-analyzed after the change from Vulnerability to Hotspot is happening in RSPEC: the issue will have the type Vulnerability, whereas the associated issue will be a Hotspot.

      Omitted

      • PRs should not be decorated with Hotspots. In the interim, Hotspots may appear on the Issues page of a PR or short-lived branch, but will disappear after MMF-1266 with no current intention to provide those security reports for PRs or short-lived branches.

      HOW

      Scanner side

      • SQ Plugin API 7.3 will add a new issue type on the Sensor API: SECURITY_HOTSPOT. Analyzers will have to handle manually the backward compatibility (if SQ < 7.3, then fallback to VULNERABILITY)
      • TBC: ActiveRules with type SECURITY_HOTSPOT should be automatically excluded when analyzing a short living branch or a PR (and also in preview mode?)

      CE side

      • Issue tracking should not automatically reopen "To Review Manual Vulnerability" (=issue type = VULNERABILITY and rule type = SECURITY_HOTSPOT and status = resolved/fixed)
      • Don't count hotspots in (new) issue count measures
      • Add 2 new metrics security_hotspots and new_security_hotspots
      • Don't send notification for new hotposts

      Web side / WS

      • add a new permission
      • Prevent to change issue type from bug/code_smell/vulnerability to hotspot
      • Prevent to change issue type from hotspot to anything else

      The issue transition workflow is common to all issue types. We will have to customize it.

      Functional status Technical issue type Technical issue status Technical issue resolution
      Open Hotspot SECURITY_HOTSPOT open  
      Reopen Hotspot SECURITY_HOTSPOT reopen  
      WontFix Hotspot SECURITY_HOTSPOT resolved wontfix
      Open Manual Vulnerability VULNERABILITY open  
      To Review HotSpot SECURITY_HOTSPOT resolved fixed
      Functional transition Technical implementation
      Security Auditor detect a vulnerability Change issue type to 'VULNERABILITY' and status to 'open'
      Developer "request review" Change issue status to 'resolved/fixed'
      Security auditor accept the fix Change issue type to 'SECURITY_HOTSPOT' status to 'resolved/wontfix'
      Security auditor reject the fix Change issue status to 'reopen' (or simply 'open' ?)

      DB

      • Add a new column on issues to differentiate manual from automatic vulnerabilities

      SonarLint

      • /batch/issues should not return SECURITY_HOTSPOTS (and maybe no manual vulnerabilities)
      • TODO : update SLCORE to not enable hotspot rules

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                alexandre.gigleux Alexandre Gigleux
                Reporter:
                alexandre.gigleux Alexandre Gigleux
              • Votes:
                0 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: