ClickHouse / ClickHouse/ClickHouse

`accept-invalid-certificate` config check uses `has()` instead of `getBool()` — explicit 0 still accepts certificates

Open
#104,512 0 comments 0 reactions 1 assignee Claimed by @azat View on GitHub
bug comp-cli
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

_Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment._

_Retrospective finding from a historical scan of [PR #65238](https://github.com/ClickHouse/ClickHouse/pull/65238) (merged 2024-07-17). Confirmed on current codebase — close with a note if already fixed._

### Describe what's wrong

When a user explicitly sets `0` in their client config file, the client still accepts invalid certificates instead of rejecting them.

**Root cause:** Client.cpp:292: `if (config().has("accept-invalid-certificate"))` only checks if the key exists, not whether its boolean value is true. So `0` triggers the acceptance logic.

**Why we believe this is a bug:** Client::initialize (Client.cpp:292) → config().has("accept-invalid-certificate") checks key existence, not value → Lines 294-295 set AcceptCertificateHandler even when value is 0

**Affected locations:**
- `programs/client/Client.cpp:292` — accept-invalid-certificate config check

**Impact:** Users who explicitly set `0` intending to reject invalid certificates will have their certificates accepted, potentially creating a security vulnerability where self-signed or invalid certificates are trusted when they should not be.

### Does it reproduce on most recent release?

Yes — confirmed on current `master` (commit `5068a189c05`).

### How to reproduce

```python
Run: cd tests/integration && CLICKHOUSE_TESTS_SERVER_BIN_PATH=../../build/programs/clickhouse CLICKHOUSE_TESTS_BASE_CONFIG_DIR=../../programs/server pytest test_accept_invalid_certificate/test_explicit_zero.py -v -s
```

### Expected behavior

```
Exception: certificate verify failed
```

### Error message and/or stacktrace

```
Connection succeeded with output '1\n' when it should have failed with 'certificate verify failed'
```

### Additional context

**Suggested fix:** Change line 292 from `if (config().has("accept-invalid-certificate"))` to `if (config().getBool("accept-invalid-certificate", false))`

**Analysis details:** Confidence HIGH | Severity P2 | Testability: `INTEGRATION_TEST`

Found during automated review of [PR #65238](https://github.com/ClickHouse/ClickHouse/pull/65238).

---
_ClickGapAI · Confidence: HIGH · Severity: P2 · Finding: `h_pr65238_001`_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.