bneradt commented on code in PR #13711:
URL: https://github.com/apache/trafficserver/pull/13711#discussion_r4074910094
##########
src/iocore/net/SSLConfig.cc:
##########
@@ -463,9 +464,12 @@ SSLConfigParams::initialize(ConfigContext ctx)
// SSL session cache configurations
ssl_origin_session_cache =
RecGetRecordInt("proxy.config.ssl.origin_session_cache.enabled").value_or(0);
ssl_origin_session_cache_size =
RecGetRecordInt("proxy.config.ssl.origin_session_cache.size").value_or(0);
+ ssl_origin_session_max_size =
+
RecGetRecordInt("proxy.config.ssl.origin_session_cache.max_session_size").value_or(SSL_DEFAULT_MAX_ORIG_SESSION_SIZE);
Review Comment:
[P2] Enforce the size range before publishing the cache limit
The RECC_INT range is enforced by SetRecordFromYAMLNode, but it does not
bound every configuration path: initialize_record() in RecordsConfigUtils.cc
applies RecConfigOverrideFromEnvironment() and registers the converted value
without calling RecordValidityCheck(). With this record omitted from
records.yaml, PROXY_CONFIG_SSL_ORIGIN_SESSION_CACHE_MAX_SESSION_SIZE=1048576
therefore reaches this assignment unchanged; -1 also reaches it and becomes
SIZE_MAX when published to origin_session_max_size. insert_session() then
allows sessions above the intended 65536-byte ceiling into SSLSessionDup() and
its alloca(len), defeating the stack-allocation bound this change relies on.
Please validate the RecInt against [4096, 65536] before narrowing/publishing it
(reject or fall back on invalid values), and cover an out-of-range environment
override.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
