Skip to content

fix(caddy): parse thread and request counts as int - #2722

Merged
dunglas merged 1 commit into
mainfrom
fix/caddy-int-overflow-32bit
Oct 16, 2026
Merged

dunglas merged 1 commit into
mainfrom
fix/caddy-int-overflow-32bit

Conversation

@dunglas

@dunglas dunglas commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

num_threads, max_threads, max_requests, the worker num and max_threads options, and php-server --worker file,N are parsed with strconv.ParseUint(s, 10, 32) and then converted to int. On 32-bit platforms (we ship linux/386, linux/arm/v6 and linux/arm/v7 images), values between 2^31 and 2^32-1 wrap to negative numbers: max_threads 4294967295 becomes -1, which means auto.

These values are now parsed with strconv.Atoi through a parseNonNegativeInt helper. Nothing gets converted, so nothing can wrap, and out-of-range input returns a range error on every platform. Negative values are still rejected explicitly, because the runtime treats them as sentinels (auto thread count, default worker count). 64-bit builds accept the same values as before.

Reported by CodeQL (go/incorrect-integer-conversion) while setting it up in #2720.

@dunglas dunglas changed the title fix(caddy): reject thread and request counts that overflow int on 32-bit fix(caddy): parse thread and request counts as int Oct 16, 2026
@dunglas
dunglas force-pushed the fix/caddy-int-overflow-32bit branch from 6ac3398 to 868a6b7 Compare October 16, 2026 12:22
@dunglas
dunglas marked this pull request as ready for review October 16, 2026 12:27

@AlliBalliBaba AlliBalliBaba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caddy seems to currently generally swallow error messages on startup (probably already fixed here?)

@dunglas
dunglas merged commit 609b44c into main Oct 16, 2026
41 checks passed
@dunglas
dunglas deleted the fix/caddy-int-overflow-32bit branch October 16, 2026 21:58
dunglas added a commit that referenced this pull request Oct 21, 2026
CodeQL has not analyzed the repository since October 2024. Default setup
is disabled, and its last C/C++ run failed because autobuild can't find
the PHP headers.

This adds an advanced setup workflow that installs ZTS PHP through the
existing `setup-php` action, exports `php-config --includes` as
`CGO_CFLAGS`, and builds both the root module and `caddy/` so the
tracers see the C and Go compilations. It covers `actions`, `c-cpp` and
`go` on pull requests, pushes to `main`, and weekly.

Notes:

- The Go build cache is disabled for this job: a warm cache skips the
compiler invocations CodeQL traces.
- `caddy/` is built from its own `working-directory` because the Go
tracer doesn't recognize `go -C`.
- CodeQL's Go extractor records files that use cgo (`frankenphp.go`,
`worker.go`, …) under their generated copies in `GOCACHE`, so Go alerts
in those files are dropped. C sources and pure Go files (including the
whole Caddy module) are covered.
- GitHub default setup must stay disabled, otherwise the two
configurations conflict.

Running the default suites locally on top of this branch reports two
issues, fixed separately:

- #2721 (`go/zipslip`,
https://github.com/php/frankenphp/security/code-scanning/1)
- #2722
(`go/incorrect-integer-conversion`, 7 occurrences in `caddy/`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants