Skip to content

Conversation

@khaledalam
Copy link

@khaledalam khaledalam commented Jan 10, 2026

Description

Fixes #16878

Problem

gmp_fact() was crashing with "GNU MP: Cannot allocate memory" abort when passed extremely large values (e.g., 2^50+1). The function calls mpz_fac_ui() which:

  1. Accepts an unsigned long parameter
  2. Can trigger massive memory allocations for large inputs, causing GMP to abort the process
  3. Had a TODO comment acknowledging the missing validation

Before this fix:

gmp_fact(gmp_pow(2, 50) + 1);  // Aborted (core dumped)
image

Reference: https://3v4l.org/X428B

Solution

Added proper input validation in gmp_fact() before calling mpz_fac_ui():

  1. Check if value fits in unsigned long: Uses mpz_fits_ulong_p() to ensure the GMP number can be safely converted
  2. Add practical upper limit: Caps input at 100,000 to prevent excessive memory/CPU usage
  3. Throw proper ValueError: Returns a clear error message instead of crashing

After this fix:

gmp_fact(gmp_pow(2, 50) + 1);  // ValueError: gmp_fact(): Argument #1 ($num) is too large

Changes Made

Modified:

  • ext/gmp/gmp.c: Added validation logic replacing the TODO comment
    • Validates mpz_fits_ulong_p(gmpnum) to prevent truncation/overflow
    • Validates mpz_cmp_ui(gmpnum, 100000) > 0 to prevent excessive resource usage

Added:

  • ext/gmp/tests/gh16878.phpt: Test case covering:
    • Very large values that previously crashed (2^50+1, 1 trillion)
    • Normal values that should work (100)

Why the 100,000 cap?

Factorial growth is astronomical:

  • 100! = 158 digits
  • 100,000! = ~456,574 digits
  • Values beyond 100,000 can consume excessive memory and CPU
  • The cap prevents accidental DoS-style usage while allowing practical computations
  • Consistent with PHP's philosophy of having resource limits on expensive operations

Error Behavior

Input Behavior
Negative value ValueError: must be greater than or equal to 0
Value > ULONG_MAX (GMP input) ValueError: is too large
Value > 100,000 ValueError: is too large
Valid value (0-100,000) Computes factorial successfully

@khaledalam khaledalam requested a review from Girgias as a code owner January 10, 2026 20:21
@khaledalam khaledalam marked this pull request as draft January 10, 2026 20:25
@khaledalam khaledalam changed the title ext/gmp: Prevent overflow in gmp_fact() for large GMP inputs (GH-16878) Fix GH-16878: Prevent overflow in gmp_fact() for large GMP inputs (ext/gmp) Jan 10, 2026
@khaledalam khaledalam marked this pull request as ready for review January 10, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core dumped when allocate a huge memory

1 participant