Skip to content

Improve Improvers #701

Open
Open

Description

@Hritik14

Improvers are constraining.

For eg: Improving reference id to reference URL, improving vuln data (not an advisory).
The problem is with both interesting_advisories and get_inferences where both of them expect AdvisoryData.
Improvers cloud be split in

  • A condition which checks if this improver runs or not
  • then a handling thing.

TOCTOU conditions are also present, we check at interesting_advisories and use the value at get_inferences. Could do https://docs.djangoproject.com/en/4.0/ref/models/querysets/#select-for-update to avoid TOCTOU closer where things are going to change. This would mean returning a QuerySet from interesting_advisories might not be an ideal case.

Current implementation could become a subclass which is a advisory based improver.

(via: https://github.com/nexB/vulnerablecode/wiki/WeeklyMeetings#meeting-on-tuesday-2022-04-19-at-1000-utc)

Activity

  1. Hritik14 commented on Apr 27, 2022

    @Hritik14
    CollaboratorAuthor

    Current problem

    • Inference class is not very scalable for different types of improvements
      • Eg: Improver to figure out CVE from a given set of references by their urls.
    • There exists a TOCTOU condition over the entire QuerySet passed

    Proposed solution

    • Eliminate intermediate Inference class and allow improvers to implement their own process method which talks to the database directly.
    • Improvers should be idempotent
    • After getting a QuerySet, iterate over each element -> select_for_update one element -> update -> repeat
    • Following is the proposed python pseudocode
    class Improver:
    
        @classproperty
        def qualified_name(cls):
            """
            Fully qualified name prefixed with the module name of the improver used in logging.
            """
            return f"{cls.__module__}.{cls.__qualname__}"
    
        @property
        def interesting_queryset(self) -> QuerySet:
            """
            Return QuerySet for the advisories this improver is interested in
            """
            raise NotImplementedError
    
        def improve(self, args) -> isImproved?:
            """
            Generate and return Inferences for the given advisory data
            """
            raise NotImplementedError
    
        def process(self, args) -> isProcessed?:
            """
            Entrypoint for the improver. Consume ``interesting_queryset`` and use ``improve`` to
            process improvement.
            """
            raise NotImplementedError
    
    class AdvisoryImprover(Improver):
    
        def process(record):
            """
            An atomic transaction that updates both the Advisory (e.g. date_improved)
            and processes the given inferences to create or update corresponding
            database fields.
    
            This avoids failing the entire improver when only a single inference is
            erroneous. Also, the atomic transaction for every advisory and its
            inferences makes sure that date_improved of advisory is consistent.
            """
            # This is somewhat the same method that exists currently in the framework for processing all improvers
            # Although, it takes care of TOCTOU as stated above
            ...

    Now different improvers can be written by implementing the AdvisoryImprover method by providing bodies for improve and interesting_queryset

    This way we have a concrete design for improvers, independent ways of handling different kinds of improvements (via the process method)

    (via: https://github.com/nexB/vulnerablecode/wiki/WeeklyMeetings#meeting-on-tuesday-2022-04-26-at-1000-utc)

  2. Hritik14 commented on Feb 7, 2023

    @Hritik14
    CollaboratorAuthor

    Related: #1068

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions