Skip to content

Integer Overflow Vulnerability in gguf_fread_str() #256

Open
@Asuk4

Description

@Asuk4

Dear PowerInfer Team,

As part of our ongoing security audit, we have identified a critical integer overflow vulnerability in the gguf_fread_str() function that could lead to memory allocation issues and potential security risks.

Mechanism of Vulnerability

  1. In gguf_fread_str(), the function reads a size value p->n directly from an untrusted input file.
  2. This value is then used in calloc(p->n + 1, 1) without any validation or overflow checks.
  3. If an attacker provides a malicious input where p->n is SIZE_MAX, the addition of 1 will cause an integer overflow, resulting in calloc(0, 1).
  4. This could lead to either:
    • A zero-byte allocation, causing subsequent buffer overflow when writing data
    • Excessive memory allocation if the implementation handles the overflow differently

Relevant Code Snippets

ggml.c - gguf_fread_str()

static bool gguf_fread_str(FILE * file, struct gguf_str * p, size_t * offset) {
    p->n    = 0;
    p->data = NULL;

    bool ok = true;

    ok = ok && gguf_fread_el(file, &p->n,    sizeof(p->n), offset); p->data = calloc(p->n + 1, 1);
    ok = ok && gguf_fread_el(file,  p->data, p->n,         offset);

    return ok;
}

ggml.c - gguf_fread_el()

static bool gguf_fread_el(FILE * file, void * dst, size_t size, size_t * offset) {
    const size_t n = fread(dst, 1, size, file);
    *offset += n;
    return n == size;
}

Vulnerability Analysis

  1. Input Source: The vulnerability originates from reading untrusted input via gguf_fread_el() into p->n.
  2. Missing Validation: No bounds checking is performed on p->n before using it in memory allocation.
  3. Arithmetic Risk: The expression p->n + 1 in calloc() is vulnerable to integer overflow.
  4. Direct Propagation: The tainted value flows directly from input to allocation without sanitization.
  5. Unconditional Flow: All execution paths reach the vulnerable allocation without intermediate constraints.

Suggested Action

The code should be modified to include proper validation and overflow checks:

  1. Add bounds checking before allocation:
if (p->n > SIZE_MAX - 1) {
    return false;  // Handle error case
}
  1. Use safe arithmetic operations:
size_t alloc_size;
if (__builtin_add_overflow(p->n, 1, &alloc_size)) {
    return false;  // Handle overflow case
}
p->data = calloc(alloc_size, 1);
  1. Consider implementing a maximum size limit for the input to prevent excessive memory allocation.

This vulnerability could be exploited by an attacker to cause denial of service or potentially more severe security issues depending on how the allocated memory is used in subsequent operations.

Thank you for your attention to this matter. We are available to provide further details if needed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions