angler-fishThe Vulnerability History Project

CVE-2019-19070
aka Missing fail safe

This vulnerability revolves around the spi_gpio_probe function. An spi gpio is basically having multiple general purpose input and outputs(gpio) included in one spi serial port interface, basically what manages all the ports for input and output. These thigns can range as anything interacting with the system like a keyboard, or mouse. So basically the specific error here is that sometimes the action for the spi_gpio_probe function doesn't go through, it has a failure path, with this failure path there are memory leaks since spi_master_put() isn't called. So we have to make sure this is called regardless of wether or not the spi_gpio_probe function is called.


I believe the mistakes involved were related to defaulting the code The code ahd null statemetns in place, making sure nothing would happen before then there was a fix to it, and this propesed fix cause dthe vulnerability. By not ignoring any checks to the code, in the first place we could have had a better security. Then when we went to repalce those null checks, we saw that the code was going to call status no matter what, especially when a failure path is occifung in the spi gpio probe function. A failure is not stopping our system and letting the code through Leaking informaiton through, somethign we needed to resolve by adding a forgotten check
  • Commits
  • Files Patched
    • Vulnerability-Contributing Commit
    1
    2
    3
    4
    5
    6
    7
    8
    9
    10
    11
    12
    13
    14
    15
    16
    17
    18
    19
    20
    21
    22
    23
    24
    25
    26
    27
    28
    29
    30
    31
    32
    33
    34
    35
    36
    37
    38
    39
    40
    41
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    76
    77
    78
    79
    80
    81
    82
    83
    84
    85
    86
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    101
    102
    103
    104
    105
    106
    107
    108
    109
    110
    111
    112
    113
    114
    115
    116
    117
    118
    119
    120
    121
    122
    123
    124
    125
    126
    127
    128
    129
    130
    131
    132
    133
    134
    135
    136
    137
    138
    139
    140
    141
    142
    143
    144
    145
    146
    147
    148
    149
    150
    151
    152
    153
    154
    155
    156
    157
    158
    159
    160
    161
    162
    163
    164
    165
    166
    167
    168
    169
    170
    171
    172
    173
    174
    175
    176
    177
    178
    179
    180
    181
    182
    183
    184
    185
    186
    187
    188
    189
    190
    191
    192
    193
    194
    195
    196
    197
    198
    199
    200
    201
    202
    203
    204
    205
    206
    207
    208
    209
    210
    211
    212
    213
    214
    215
    216
    217
    218
    219
    220
    221
    222
    223
    224
    225
    226
    227
    228
    229
    230
    231
    232
    233
    234
    235
    236
    237
    238
    239
    240
    241
    242
    243
    244
    245
    246
    247
    248
    249
    250
    251
    252
    253
    254
    255
    256
    257
    258
    259
    260
    261
    262
    263
    264
    265
    266
    267
    268
    269
    270
    271
    272
    273
    274
    275
    276
    277
    278
    279
    280
    281
    282
    283
    284
    285
    286
    287
    288
    289
    290
    291
    292
    293
    294
    295
    296
    297
    298
    299
    300
    301
    302
    303
    304
    305
    306
    307
    308
    309
    310
    311
    312
    313
    314
    315
    316
    317
    318
    319
    320
    321
    322
    323
    324
    325
    326
    327
    328
    329
    330
    331
    332
    333
    334
    335
    336
    337
    338
    339
    340
    341
    342
    343
    344
    345
    346
    347
    348
    349
    350
    351
    352
    353
    354
    355
    356
    357
    358
    359
    360
    361
    362
    363
    364
    365
    366
    367
    368
    369
    370
    371
    372
    373
    374
    375
    376
    377
    378
    379
    380
    381
    382
    383
    384
    385
    386
    387
    388
    389
    390
    391
    392
    393
    394
    395
    396
    397
    398
    399
    400
    401
    402
    403
    404
    405
    406
    407
    408
    409
    410
    411
    412
    413
    414
    415
    416
    417
    418
    419
    420
    421
    422
    423
    424
    425
    426
    427
    428
    429
    430
    431
    432
    433
    434
    435
    436
    437
    438
    439
    440
    441
    442
    443
    444
    445
    446
    447
    448
    449
    450
    451
    452
    453
    454
    455
    456
    457
    458
    459
    460
    461
    462
    463
    464
    465
    466
    467
    468
    469
    470
    471
    472
    473
    474
    475
    476
    477
    478
    479
    480
    481
    482
    483
    484
    485
    486
    487
    488
    489
    490
    491
    492
    493
    494
    495
    496
    497
    498
    499
    500
    501
    502
    503
    504
    505
    506
    507
    508
    509
    510
    511
    512
    513
    514
    515
    516
    517
    518
    519
    520
    521
    522
    523
    524
    525
    526
    527
    528
    529
    530
    531
    532
    533
    534
    535
    536
    537
    538
    539
    540
    541
    542
    543
    544
    545
    546
    547
    
    CVE: CVE-2019-19070
    CWE:
    - 401
    ipc:
      note: |
        I believe communciation between inputs might relate to the
        stint/stout library so it might affect this case
      answer: true
      question: |
        Did the feature that this vulnerability affected use inter-process
        communication? IPC includes OS signals, pipes, stdin/stdout, message
        passing, and clipboard. Writing to files that another program in this
        software system reads is another form of IPC.
    
        Answer must be true or false.
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
    CVSS: CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
    bugs: []
    i18n:
      note: |
        Not really related to this vulnerability it can happen all over the world
        it doesn't matter where they're from the failure path can still be reached and
        cause a denial of service error
      answer: false
      question: |
        Was the feature impacted by this vulnerability about internationalization
        (i18n)?
    
        An internationalization feature is one that enables people from all
        over the world to use the system. This includes translations, locales,
        typography, unicode, or various other features.
    
        Answer should be true or false
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
    vccs:
    - note: Discovered automatically by archeogit.
      commit: 8b797490b4db09492acda4b4a4a4355d2311a614
    - note: Discovered automatically by archeogit.
      commit: 249e2632dcd0509b8f8f296f5aabf4d48dfd6da8
    fixes:
    - note: |
        We just have to add a check after the function spi_gpio_probe is finished to cleanup
        what we have, at the end there is a function called devm_add_action_or_reset() which puts
        the data we have throught the put function.
      commit: 79567c1a321e7396e734135d56950311dde07db2
    - note: |
        Taken from NVD references list with Git commit. If you are
        curating, please fact-check that this commit fixes the vulnerability and replace this comment with 'Manually confirmed'
      commit: d3b0ffa1d75d5305ebe34735598993afbb8a869d
    vouch:
      note: |
        Well there were multiple people singing off on each other for the vcc.
        As well as multiple people signing off on each otehr for the fix commit
      answer: true
      question: |
        Was there any part of the fix that involved one person vouching for
        another's work?
    
        This can include:
          * signing off on a commit message
          * mentioning a discussion with a colleague checking the work
          * upvoting a solution on a pull request
    
        Answer must be true or false.
        Write a note about how you came to the conclusions you did, regardless of what your answer was.
    bounty:
      amt: 
      url: 
      announced: 
    lessons:
      yagni:
        note: |
          We were using a fix that was provided to by a previous working version
          but then
        applies: true
      question: |
        Are there any common lessons we have learned from class that apply to this
        vulnerability? In other words, could this vulnerability serve as an example
        of one of those lessons?
    
        Leave "applies" blank or put false if you did not see that lesson (you do
        not need to put a reason). Put "true" if you feel the lesson applies and put
        a quick explanation of how it applies.
    
        Don't feel the need to claim that ALL of these apply, but it's pretty likely
        that one or two of them apply.
    
        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.
      serial_killer:
        note: 
        applies: 
      complex_inputs:
        note: 
        applies: 
      distrust_input:
        note: |
          these inputs can be faulty and result in a failure path
          causign this vulnerability
        applies: true
      least_privilege:
        note: 
        applies: false
      native_wrappers:
        note: 
        applies: false
      defense_in_depth:
        note: 'additional forgotten check added
    
          '
        applies: true
      secure_by_default:
        note: 
        applies: 
      environment_variables:
        note: 
        applies: 
      security_by_obscurity:
        note: |
          kind of, lacking in this case nobody really would think to mess around with
          the gpio inputs
        applies: true
      frameworks_are_optional:
        note: 
        applies: false
    reviews: []
    sandbox:
      note: |
        This is not related to permissions it's more of a lack of error handling
        in a function since failure path, anyone could have called the function really
        since it's  part of input/outputs
      answer: false
      question: |
        Did this vulnerability violate a sandboxing feature that the system
        provides?
    
        A sandboxing feature is one that allows files, users, or other features
        limited access. Vulnerabilities that violate sandboxes are usually based on
        access control, checking privileges incorrectly, path traversal, and the
        like.
    
        Answer should be true or false
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
    upvotes: 
    CWE_note: |
      CWE as registered in the NVD. If you are curating, check that this
      is correct and replace this comment with "Manually confirmed".
    mistakes:
      answer: |
        I believe the mistakes involved were related to defaulting the code
        The code ahd null statemetns in place, making sure nothing would happen before
        then there was a fix to it, and this propesed fix cause dthe vulnerability.
        By not ignoring any checks to the code, in the first place we could have had
        a better security.
    
        Then when we went to repalce those null checks, we saw that the code was going to
        call status no matter what, especially when a failure path is occifung in the spi
        gpio probe function. A failure is not stopping our system and letting the code through
        Leaking informaiton through, somethign we needed to resolve by adding a forgotten check
      question: |
        In your opinion, after all of this research, what mistakes were made that
        led to this vulnerability? Coding mistakes? Design mistakes?
        Maintainability? Requirements? Miscommunications?
    
        There can, and usually are, many mistakes behind a vulnerability.
    
        Remember that mistakes can come in many forms:
        * slip: failing to complete a properly planned step due to inattention
                  e.g. wrong key in the ignition
                  e.g. using < instead of <=
        * lapse: failing to complete a properly planned step due to memory failure
                  e.g. forgetting to put car in reverse before backing up
                  e.g. forgetting to check null
        * planning error: error that occurs when the plan is inadequate
                  e.g. getting stuck in traffic because you didn't consider the
                       impact of the bridge closing
                  e.g. calling the wrong method
                  e.g. using a poor design
    
        These are grey areas, of course. But do your best to analyze the mistakes
        according to this framework.
    
        Look at the CWE entry for this vulnerability and examine the mitigations
        they have written there. Are they doing those? Does the fix look proper?
    
        Write a thoughtful entry here that people in the software engineering
        industry would find interesting.
    nickname: Missing fail safe
    subsystem:
      name:
      - gpio
      note: Honestly can't tell difference between subsystems and specifications
      question: |
        What subsystems was the mistake in? These are WITHIN linux kernel
    
        Determining the subsystem is a subjective task. This is to help us group
         similar vulnerabilities, so choose a subsystem that other vulnerabilities would be in. Y
    
        Some areas to look for pertinent information:
          - Bug labels
          - Directory names
          - How developers refer to an area of the system in comments,
            commit messages, etc.
    
        Look at the path of the source code files code that were fixed to get
        directory names. Look at comments in the code. Look at the bug reports how
        the bug report was tagged.
    
        Example linux kernel subsystems are:
          * drivers
          * crypto
          * fs
          * net
          * lib
    
        Name should be:
          * all lowercase English letters
          * NOT a specific file
          * can have digits, and _-@/
    
        Can be multiple subsystems involved, in which case you can make it an array
        e.g.
            name: ["subsystemA", "subsystemB"] # ok
            name: subsystemA # also ok
    discovered:
      answer: |
        I believe it was found during a school session. Navid Emamdoost is the founder
        of this vulnerability and in the patch report here
    
        https://lore.kernel.org/all/20190930205241.5483-1-navid.emamdoost@gmail.com/
    
        It states that he(Navid Ermandoost) is cc'ing people at the university of Minnesota Twin Cities,
        umn and I think he was using this code to test a gpio input and a fialure
        path occured causing this vulnerability.
      contest: false
      question: |
        How was this vulnerability discovered?
    
        Go to the bug report and read the conversation to find out how this was
        originally found. Answer in longform below in "answer", fill in the date in
        YYYY-MM-DD, and then determine if the vulnerability was found by a Google
        employee (you can tell from their email address). If it's clear that the
        vulenrability was discovered by a contest, fill in the name there.
    
        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.
      automated: false
      developer: true
    discussion:
      note: |
        Judging by the official linux fix link
        https://lore.kernel.org/all/20190930205241.5483-1-navid.emamdoost@gmail.com/
        I don't think there's much of a discussion toher than some replies by
        some devs stating that there is a problem, how it occured,
        and what they did to fix it.
      question: |
        Was there any discussion surrounding this?
    
        A discussion can include debates, disputes, or polite talk about how to
        resolve uncertainty.
    
        Example include:
          * Is this out of our scope?
          * Is this a security?
          * How should we fix this?
    
        Just because you see multiple comments doesn't mean it's a discussion.
        For example:
          * "Fix line 10". "Ok" is not what we call a discussion
          * "Ping" (reminding people)
    
        Check the bugs reports, pull requests, and mailing lists archives.
    
        These answers should be boolean.
          discussed_as_security: true or false
          any_discussion: true or false
    
        Put any links to disagreements you found in the notes section, or any other
        comment you want to make.
      any_discussion: false
      discussed_as_security: false
    stacktrace:
      note: |
        I couldn't find any based on the rpevious commits and basedon the results
        I believe that this was a failure path result, form having a denial of service
        error, so a psuedo stack trace can exist by having the latest instance
        at whcih the code is running in the spi gpio probe function
      question: |
        Are there any stacktraces in the bug reports?
    
        Secondly, if there is a stacktrace, is the fix in the same file that the
        stacktrace points to?
    
        If there are no stacktraces, then both of these are false - but be sure to
        mention where you checked in the note.
    
        Answer must be true or false.
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
      any_stacktraces: false
      stacktrace_with_fix: false
    description: |
      This vulnerability revolves around the spi_gpio_probe function. An spi gpio
      is basically having multiple general purpose input and outputs(gpio) included in
      one spi serial port interface, basically what manages all the ports for input
      and output. These thigns can range as anything interacting with the system like a
      keyboard, or mouse.
    
      So basically the specific error here is that sometimes the action for the spi_gpio_probe
      function doesn't go through, it has a failure path, with this failure path there
      are memory leaks since spi_master_put() isn't called. So we have to make sure this is called
      regardless of wether or not the spi_gpio_probe function is called.
    unit_tested:
      fix: false
      code: false
      question: |
        Were automated unit tests involved in this vulnerability?
        Was the original code unit tested, or not unit tested? Did the fix involve
        improving the automated tests?
    
        For code: and fix: - your answer should be boolean.
    
        For the code_answer below, look not only at the fix but the surrounding
        code near the fix in related directories and determine if and was there were
        unit tests involved for this subsystem.
    
        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.
      fix_answer: 
      code_answer: 
    reported_date: '2019-11-20'
    specification:
      note: spi, spi-gpio
      answer: true
      instructions: |
        Is there mention of a violation of a specification? For example, the POSIX
        spec, an RFC spec, a network protocol spec, or some other requirements
        specification.
    
        Be sure to check the following artifacts for this:
          * bug reports
          * security advisories
          * commit message
          * mailing lists
          * anything else
    
        The answer field should be boolean. In answer_note, please explain
        why you come to that conclusion.
    announced_date: '2019-11-18'
    curation_level: 2
    published_date: '2019-11-18'
    forgotten_check:
      note: |
        there is a status check if there is a status from the cleanup fucntion menaing
        it failed it will return that status
      answer: true
      question: |
        Does the fix for the vulnerability involve adding a forgotten check?
    
        A "forgotten check" can mean many things. It often manifests as the fix
        inserting an entire if-statement or a conditional to an existing
        if-statement. Or a call to a method that checks something.
    
        Example of checks can include:
          * null pointer checks
          * check the current role, e.g. root
          * boundary checks for a number
          * consult file permissions
          * check a return value
    
        Answer must be true or false.
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
    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
      https://cwe.mitre.org/data/definitions/699.html for the Software Development
      view of the vulnerabilities. We also recommend the tool
      http://www.cwevis.org/viz to help see how the classifications work.
    
      If you have anything to note about why you classified it this way, write
      something in CWE_note. This field is optional.
    
      Just the number here is fine. No need for name or CWE prefix. If more than one
      apply here, then place them in an array like this
        CWE: ["123", "456"] # this is ok
        CWE: [123, 456]     # also ok
        CWE: 123            # also ok
    autodiscoverable:
      note: |
        There are multiple tests needed to check failure apths for the spi
        gpi probe function and these paths probably should have a test for each
        to get code coverage, so one failure path will cause a failed test or the error
        to occur.
      answer: true
      instructions: |
        Is it plausible that a fully automated tool could have discovered
        this? These are tools that require little knowledge of the domain,
         e.g. automatic static analysis, compiler warnings, fuzzers.
    
        Examples for true answers: SQL injection, XSS, buffer overflow
    
        In systemd, the actually use OZZ Fuzz. If there's a link to it, add it here.
    
        Examples for false: RFC violations, permissions issues, anything
        that requires the tool to be "aware" of the project's
        domain-specific requirements.
    
        The answer field should be boolean. In answer_note, please explain
        why you come to that conclusion.
    vcc_instructions: |
      The vulnerability-contributing commits.
    
      These are found by our tools by traversing the Git Blame history, where we
      determine which commit(s) introduced the functionality.
    
      Look up these VCC commits and verify that they are not simple refactorings,
      and that they are, in fact introducing the vulnerability into the system.
      Often, introducing the file or function is where the VCC is, but VCCs can be
      anything.
    
      Place any notes you would like to make in the notes field.
    bugs_instructions: |
      What bugs are involved in this vulnerability?
    
      Please list bug IDs to https://bugzilla.kernel.org/
    
      Bug ID's can appear in several places:
        * Mentioned in commit messages
        * Mentioned in mailing list discussions
        * References from NVD entry
        * Various other places
    yaml_instructions: |
      =================
      ===YAML Primer===
      =================
      This is a dictionary data structure, akin to JSON.
      Everything before a colon is a key, and the values here are usually strings
      For one-line strings, you can just use quotes after the colon
      For multi-line strings, as we do for our instructions, you put a | and then
      indent by two spaces
    
      For readability, we hard-wrap multi-line strings at 80 characters. This is
      not required, but appreciated.
    fixes_instructions: |
      Please put the commit hash in "commit" below.
    
      This must be a git commit hash from the systemd source repo, a  40-character
      hexademical string/
    
      Place any notes you would like to make in the notes field.
    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
      was wrong. Otherwise, leave it blank.
    interesting_commits:
      commits:
      - note: |
          I thought it was weird how there is a cleanup function after the end
          but I think personally there is no need and we can just have it simplified
          with a forgotten check in the spi gpio probe function instead of having
          an additional cleanup function.
        commit: 8b797490b4db09492acda4b4a4a4355d2311a614
      - note: 
        commit: 
      question: |
        Are there any interesting commits between your VCC(s) and fix(es)?
    
        Use this to specify any commits you think are notable in some way, and
        explain why in the note.
    
        Example interesting commits:
          * Mentioned as a problematic commit in the past
            e.g. "This fixes regression in commit xys"
          * A significant rewrite in the git history
          * Other commits that fixed a similar issue as this vulnerability
          * Anything else you find interesting.
    order_of_operations:
      note: There is just another forgotten check at the bottom
      answer: false
      question: |
        Does the fix for the vulnerability involve correcting an order of
        operations?
    
        This means the fix involves moving code around or changing the order of
        how things are done.
    
        Answer must be true or false.
        Write a note about how you came to the conclusions you did, regardless of
        what your answer was.
    curated_instructions: |
      If you are manually editing this file, then you are "curating" it.
    
      Set the version number that you were given in your instructions.
    
      This will enable additional editorial 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 properly updated.
    upvotes_instructions: |
      For the first round, ignore this upvotes number.
    
      For the second round of reviewing, you will be giving a certain amount of
      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.
    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.
    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.
    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.
    
      This is not the same as published date in the NVD - that is below.
    
      Please enter your date in YYYY-MM-DD format.
    published_instructions: |
      Is there a published fix or patch date for this vulnerability?
      Please enter your date in YYYY-MM-DD format.
    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.
    
      Rewrite this description IN YOUR OWN WORDS. Make it interesting and easy to
      read to anyone with some programming experience. We can always pull up the NVD
      description later to get more technical.
    
      Try to still be specific in your description, but remove project-specific
      stuff. Remove references to versions, specific filenames, and other jargon
      that outsiders to this project would not understand. Technology like "regular
      expressions" is fine, and security phrases like "invalid write" are fine to
      keep too.
    
      Your target audience is people just like you before you took any course in
      security
    

    See a mistake? Is something missing from our story? We welcome contributions! All of our work is open-source and version-controlled on GitHub. You can curate using our Curation Wizard.

    Use our Curation Wizard

    Or go to GitHub

    • There are no articles here... yet

    Timeline

    Hover over an event to see its title.
    Click on the event to learn more.
    Filter by event type with the buttons below.

    expand_less