Buffer Overflow (OOB Write/Read) in `Hardware` and `Revision` Parsing

[mhansen]

I found this while trying to figure out why I was getting errors when running under strict checks (#402). While this wasn't the cause of my errors, I think it's a real (separate) bug.

The /proc/cpuinfo parser in src/arm/linux/cpuinfo.c has a logic error when handling Hardware and Revision values that are equal to or longer than the maximum buffer size (CPUINFO_HARDWARE_VALUE_MAX and CPUINFO_REVISION_VALUE_MAX, both 64).

This leads to:

1. Out-of-bounds write of 1 byte (null terminator) if the value length is exactly 64.
2. Lack of null-termination (potentially leading to out-of-bounds reads later) if the value length is greater than 64.

Affected Code

In src/arm/linux/cpuinfo.c:

cpuinfo/src/arm/linux/cpuinfo.c

Lines 873 to 885 in ae54436

	} else if (memcmp(line_start, "Hardware", key_length) == 0) {
	size_t value_length = value_end - value_start;
	if (value_length > CPUINFO_HARDWARE_VALUE_MAX) {
	cpuinfo_log_warning(
	"length of Hardware value \"%.*s\" in /proc/cpuinfo exceeds limit (%d): truncating to the limit",
	(int)value_length,
	value_start,
	CPUINFO_HARDWARE_VALUE_MAX);
	value_length = CPUINFO_HARDWARE_VALUE_MAX;
	} else {
	state->hardware[value_length] = '\0';
	}
	memcpy(state->hardware, value_start, value_length);

Scenario 1: value_length == 64 (equal to CPUINFO_HARDWARE_VALUE_MAX)

• The if condition value_length > 64 is false.
• The else branch is executed: state->hardware[64] = '\0';.
• Since state->hardware is char hardware[64], index 64 is out of bounds (valid indices are 0-63). This is a 1-byte OOB write.

Scenario 2: value_length > 64 (e.g., 70)

• The if condition is true.
• value_length is truncated to 64.
• The else branch is skipped, so no null terminator is written.
• memcpy copies 64 bytes to state->hardware.
• state->hardware is left non-null-terminated. If it is later read as a C-string, it will cause an OOB read.

The same logic error exists for the Revision field.

Suggested Fix

Check for value_length >= LIMIT. If so, truncate to LIMIT - 1 and warn. Always write the null terminator at value_length after truncation/copying.

--- a/src/arm/linux/cpuinfo.c
+++ b/src/arm/linux/cpuinfo.c
@@ -872,31 +873,29 @@
 				/* BogoMIPS is useless, don't parse */
 			} else if (memcmp(line_start, "Hardware", key_length) == 0) {
 				size_t value_length = value_end - value_start;
-				if (value_length > CPUINFO_HARDWARE_VALUE_MAX) {
+				if (value_length >= CPUINFO_HARDWARE_VALUE_MAX) {
 					cpuinfo_log_warning(
 						"length of Hardware value \"%.*s\" in /proc/cpuinfo exceeds limit (%d): truncating to the limit",
 						(int)value_length,
 						value_start,
-						CPUINFO_HARDWARE_VALUE_MAX);
-					value_length = CPUINFO_HARDWARE_VALUE_MAX;
-				} else {
-					state->hardware[value_length] = '\0';
+						CPUINFO_HARDWARE_VALUE_MAX - 1);
+					value_length = CPUINFO_HARDWARE_VALUE_MAX - 1;
 				}
+				state->hardware[value_length] = '\0';
 				memcpy(state->hardware, value_start, value_length);
 				cpuinfo_log_debug(
 					"parsed /proc/cpuinfo Hardware = \"%.*s\"", (int)value_length, value_start);
 			} else if (memcmp(line_start, "Revision", key_length) == 0) {
 				size_t value_length = value_end - value_start;
-				if (value_length > CPUINFO_REVISION_VALUE_MAX) {
+				if (value_length >= CPUINFO_REVISION_VALUE_MAX) {
 					cpuinfo_log_warning(
 						"length of Revision value \"%.*s\" in /proc/cpuinfo exceeds limit (%d): truncating to the limit",
 						(int)value_length,
 						value_start,
-						CPUINFO_REVISION_VALUE_MAX);
-					value_length = CPUINFO_REVISION_VALUE_MAX;
-				} else {
-					state->revision[value_length] = '\0';
+						CPUINFO_REVISION_VALUE_MAX - 1);
+					value_length = CPUINFO_REVISION_VALUE_MAX - 1;
 				}
+				state->revision[value_length] = '\0';
 				memcpy(state->revision, value_start, value_length);

[mhansen]

Here's a repro:

#include <stdio.h>
#include <string.h>
#include <cpuinfo-mock.h>
#include <arm/linux/api.h>

static struct cpuinfo_mock_file mock_files[] = {
    {
        .path = "/proc/cpuinfo",
        .size = 0, // Will be initialized in main
        .content =
            "Hardware\t: "
            "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n",
    },
    {.path = NULL}};

int main() {
  mock_files[0].size = strlen(mock_files[0].content);
  cpuinfo_mock_filesystem(mock_files);

  char hardware[64];
  char revision[9];
  struct cpuinfo_arm_linux_processor processors[1];

  printf("Calling cpuinfo_arm_linux_parse_proc_cpuinfo...\n");
  cpuinfo_arm_linux_parse_proc_cpuinfo(hardware, revision, 1, processors);

  printf("Hardware parsed: %s\n", hardware);
  return 0;
}

cc_test(
    name = "repro_cpuinfo",
    srcs = ["repro_cpuinfo.c"],
    linkstatic = True,
    copts = [
        "-O0",
        "-DCPUINFO_MOCK=1",
        "-Wno-vla",
        "-Ithird_party/cpuinfo/src",
        "-Ithird_party/cpuinfo/include",
    ],
    deps = [
        "//third_party/cpuinfo:cpuinfo_mock",
    ],
)

blaze test \
  --cpu=arm \
  --config=asan \
  :repro_cpuinfo