Skip to content

Resolve vulnerability: CWE-120 in picow_blink.c

DarwinJSrequested to merge
remediate/cwe-120-in-picow-blink-c-D20250724T212529 into 5-robot3

Vulnerability finding detected in merge request: Resolve Vuln (!7) • Unassigned

AI GENERATED FIX

The suggested code changes were generated by GitLab Duo Vulnerability Resolution, an AI feature. Use this feature with caution. Before you run a pipeline or apply the code changes, carefully review and test them, to ensure that they solve the vulnerability.

The large language model that generated the suggested code changes was provided with the entire file that contains the vulnerable lines of code. It is not aware of any functionality outside of this context.

Please see our documentation for more information about this feature.

Description:

This code writes past the end of the buffer pointed to by new_loc. - new_loc evaluates to malloc(strlen(s)) + 1picow_blink.c:36. - strcpy() writes to the byte at an offset that is the length of the string pointed to by s, plus 1 from the beginning of the buffer pointed to by new_loc. - The offset exceeds the capacity. - The length of the string pointed to by s, plus 1 is no less than 1. - The capacity of the buffer pointed to by new_loc, in bytes, is the length of the string pointed to by s, which is bounded below by 0. - The overrun occurs in heap memory.The issue can occur if the highlighted code executes.

Summary:

  1. The reported vulnerability is CWE-120 (Buffer Copy without Checking Size of Input), where the code allocates insufficient memory for a string copy operation. The specific issue is in the return_append_str function where malloc(strlen(s)) + 1 incorrectly adds 1 to the pointer returned by malloc, rather than allocating strlen(s) + 1 bytes.

  2. The fix corrects the memory allocation by properly grouping the expression as malloc(strlen(s) + 1). This ensures that:

    • The size calculation is performed first (strlen(s) + 1)
    • The result is passed to malloc as a single argument
    • Sufficient memory is allocated for the string plus the null terminator

The original code had a critical operator precedence issue where the + 1 was being applied to the pointer returned by malloc rather than to the size argument. This would cause malloc to allocate only strlen(s) bytes (not enough for the null terminator), and then the pointer would be incremented by 1, pointing to an invalid memory location. When strcpy was called, it would write beyond the allocated memory, causing a buffer overflow.

The fix ensures proper memory allocation with enough space for both the string content and its null terminator, preventing the buffer overflow vulnerability.

Identifiers:

  • CWE-120
  • Buffer Overrun - MisraC2023:D.4.11 - MisraC2023:D.4.1 - MisraC2023:21.17 - MisraC2023:18.2 - MisraC2023:18.1 - MisraC2023:1.3
  • MisraC2023:1.3
  • MisraC2023:18.1
  • MisraC2023:18.2
  • MisraC2023:21.17
  • MisraC2023:D.4.1
  • Misra2012:1.3
  • Misra2012:18.1
  • Misra2012:18.2
  • Misra2012:21.17
  • Misra2012:D.4.1
  • Misra2004:17.1
  • Misra2004:17.2
  • MisraC++2023:4.1.3
  • MisraC++2023:8.7.1
  • AUTOSARC++14:A27-0-2
  • AUTOSARC++14:A5-2-5
  • AUTOSARC++14:M5-0-16
  • MisraC++2008:5-0-16
  • CWE-788
  • TS17961:5.45-taintsink
  • CERT-C:ARR30-C
  • CERT-C:ARR37-C
  • CERT-C:ARR38-C
  • CERT-C:ARR39-C
  • CERT-C:ENV01-C
  • CERT-C:EXP08-C
  • CERT-C:MEM35-C
  • CERT-C:POS30-C
  • CERT-C:STR31-C
  • CERT-C:STR38-C
  • CERT-CPP:CTR50-CPP
  • CERT-CPP:CTR52-CPP
  • CERT-CPP:CTR53-CPP
  • CERT-CPP:MEM54-CPP
  • CERT-CPP:STR50-CPP
  • JSF++:211
  • DISA-6r1:V-222612
  • DISA-5r3:V-70277
  • DISA-4r3:V-70277
  • DISA-3r10:V-6165
  • A8:2021
  • A8:2017

Merge request reports