Skip to content

Resolve vulnerability: CWE-120 in picow_blink.c

DarwinJSrequested to merge
remediate/cwe-120-in-picow-blink-c-D20250729T192350 into 5-robot2

Vulnerability finding detected in merge request: All the Things for Embedded (!6) • 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:

Vulnerability Analysis and Fix

The reported vulnerability is a CWE-120 (Buffer Copy without Checking Size of Input) in the picow_blink.c file. The specific issue is in the return_append_str function where there's a buffer overflow vulnerability due to incorrect memory allocation.

The Issue

The problematic line is:

new_loc = (char *) malloc(strlen(s)) + 1;

Due to operator precedence, this code first allocates memory of size strlen(s) bytes, casts the result to (char *), and then adds 1 to the pointer address. This means:

  1. The allocation is too small (missing space for the null terminator)
  2. The pointer arithmetic is adding 1 to the address, not to the size
  3. When strcpy(new_loc, s) is called, it writes beyond the allocated buffer

The Fix

The corrected code properly allocates memory for the string plus its null terminator:

new_loc = (char *) malloc(strlen(s) + 1);

This ensures that:

  1. The size calculation includes space for the null terminator
  2. The pointer arithmetic is applied to the size, not the resulting pointer
  3. The strcpy() operation now has sufficient space to copy the entire string safely

This fix addresses the buffer overflow vulnerability while maintaining the original functionality of the code.

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