ssls: validate input arguments in import, export, and unpack

[filtede98]

Summary

This PR hardens the SSL session ticket import/export APIs and unpacking against NULL pointer dereferences and invalid arguments:

• curl_easy_ssls_import(): returns CURLE_BAD_FUNCTION_ARGUMENT if sdata is NULL or sdata_len is 0. Per curl_easy_ssls_import(3), sdata and sdata_len must always be provided; in release builds (NDEBUG), passing NULL previously bypassed DEBUGASSERT(buf) and crashed inside spack_dec8() trying to dereference *src. Passing sdata_len == 0 returned CURLE_READ_ERROR instead of CURLE_BAD_FUNCTION_ARGUMENT.
• curl_easy_ssls_export(): returns CURLE_BAD_FUNCTION_ARGUMENT when export_fn is NULL.
• Curl_ssl_session_import(): validates that sdata and sdata_len are provided before calling Curl_ssl_session_unpack().
• Curl_ssl_session_unpack(): validates ps before initializing *ps = NULL, and validates buf and buflen before parsing.
• Added regression test checks in tests/libtest/lib1678.c to verify that curl_easy_ssls_import() and curl_easy_ssls_export() return CURLE_BAD_FUNCTION_ARGUMENT on invalid arguments.

[testclutch]

Analysis of PR #23062 at 7d4a82e8:

Test 3232 failed, but it has been 76.8% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (4, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Test 3231 failed, but it has been 74.5% flaky lately, so it's probably NOT a fault of the PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (4, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

[bagder]

This is an internal function that should never be called with bad parameters. I think they are better checked with DEBUGASSERT()s that make the entire thing halt in debug builds if violated, and adds nothing to release builds.

[filtede98]

Good point. Updated to use DEBUGASSERT for the internal Curl_ssl_session_unpack() checks while keeping argument validation in the public entry points.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unpacking still relies on debug-only assertions and can dereference invalid pointers in release builds.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Validate pointers before computing buffer end and dereferencing · New

What changed in this PR

Hardens SSL session import/export and unpacking against invalid arguments, with regression coverage.

Changes:

• Rejects missing import data and export callbacks.
• Adds session-cache and unpacking input checks.
• Adds invalid-argument tests.

File	Description
tests/​libtest/​lib1678.c	Tests invalid import/export arguments.
lib/​vtls/​vtls_spack.c	Adds unpacking checks; release-safe runtime validation remains needed.
lib/​vtls/​vtls_scache.c	Validates imported session data.
lib/​easy.c	Validates public API arguments.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bagder]

pff, contrary to what I just said... I say ignore the clanker.