[cpp-sp] branch main updated: Fix object leak in read operation.
Codeberg
noreply at shibboleth.net
Thu Oct 1 14:15:18 UTC 2026
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository cpp-sp.
View the commit online:
https://codeberg.org/Shibboleth/cpp-sp/commit/a5a5fc38cd02d4e806f159321060cf31e5acd3c4
The following commit(s) were added to refs/heads/main by this push:
new a5a5fc38 Fix object leak in read operation.
a5a5fc38 is described below
commit a5a5fc38cd02d4e806f159321060cf31e5acd3c4
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Thu Oct 1 10:14:59 2026 -0400
Fix object leak in read operation.
---
shibsp/session/impl/FilesystemSessionCache.cpp | 47 +++++++++++++-------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index 13d7ed0f..1d8c1058 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -263,7 +263,7 @@ DDF FilesystemSessionCache::cache_read(
const char* client_addr
)
{
- DDF obj;
+ DDF sessionData;
time_t effective_access = 0;
string effective_path;
string base_path = m_dir + key;
@@ -282,7 +282,7 @@ DDF FilesystemSessionCache::cache_read(
// If this is the first loop iteration, we fail outright, assuming the session was removed.
if (version == effective_version) {
log(INFO_MARK, "session file (%s) does not exist", path.c_str());
- return obj;
+ return sessionData;
}
// Need to decrement back to the last known good value.
effective_version--;
@@ -316,7 +316,7 @@ DDF FilesystemSessionCache::cache_read(
log(INFO_MARK, "session (%s) expired for inactivity, timeout (%lu), last access (%s)", key, timeout, ts.c_str());
}
cache_remove(request, key);
- return obj;
+ return sessionData;
}
}
@@ -335,51 +335,52 @@ DDF FilesystemSessionCache::cache_read(
DDF out = remoting->send(in); // let this throw to caller
DDFJanitor outjanitor(out);
DDF decryptedList = out["decrypted"];
- const char* decrypted = decryptedList.islist() && decryptedList.integer() == 1 ? decryptedList.get(0).string() : nullptr;
+ const char* decrypted = (decryptedList.islist() && decryptedList.integer() == 1) ?
+ decryptedList.get(0).string() : nullptr;
if (!decrypted || !*decrypted) {
throw OperationException("No decrypted session data returned from sealer operation.");
}
istringstream source(decrypted);
- source >> obj;
+ source >> sessionData;
}
else {
- is >> obj;
+ is >> sessionData;
is.close();
}
- if (!isSessionDataValid(obj)) {
+ DDFJanitor sessionDataJanitor(sessionData);
+
+ if (!isSessionDataValid(sessionData)) {
log(ERROR_MARK, "deserialized session from file (%s) was invalid", effective_path.c_str());
throw IOException("Session data was invalid.");
}
- const char* appId = obj["app_id"].string();
+ const char* appId = sessionData["app_id"].string();
if (strcmp(applicationId, appId)) {
log(WARN_MARK, "session (%s) issued for application (%s), accessed via application (%s)", key, appId, applicationId);
- return obj.destroy();
+ return DDF();
}
if (lifetime) {
- time_t notonorafter = obj["notonorafter"].longinteger();
+ time_t notonorafter = sessionData["notonorafter"].longinteger();
if (notonorafter > 0 && notonorafter <= now) {
- obj.destroy();
if ((request && request->isPriorityEnabled(Priority::SHIB_INFO)) || m_spilog.isInfoEnabled()) {
string expired(date::format("%FT%TZ", chrono::system_clock::from_time_t(notonorafter)));
log(INFO_MARK, "session (%s) has expired per NotOnOrAfter policy (%s)", key, expired.c_str());
}
cache_remove(request, key);
- return obj;
+ return DDF();
}
- time_t start = obj["ts"].longinteger();
+ time_t start = sessionData["ts"].longinteger();
if (start + lifetime < now) {
- obj.destroy();
if ((request && request->isPriorityEnabled(Priority::SHIB_INFO)) || m_spilog.isInfoEnabled()) {
string created(date::format("%FT%TZ", chrono::system_clock::from_time_t(start)));
string expired(date::format("%FT%TZ", chrono::system_clock::from_time_t(start + lifetime)));
log(INFO_MARK, "session (%s) has expired, created (%s), expired (%s)", key, created.c_str(), expired.c_str());
}
cache_remove(request, key);
- return obj;
+ return DDF();
}
}
@@ -389,11 +390,11 @@ DDF FilesystemSessionCache::cache_read(
if (client_addr) {
const char* family = getAddressFamily(client_addr);
- const char* addr = obj[family].string();
+ const char* addr = sessionData[family].string();
if (addr) {
if (!isAddressMatch(client_addr, addr)) {
log(INFO_MARK, "session (%s) use invalid, bound to address (%s), accessed from (%s)", key, addr, client_addr);
- return obj.destroy();
+ return DDF();
}
}
else {
@@ -404,11 +405,11 @@ DDF FilesystemSessionCache::cache_read(
// the act of creating a new file version.
updateTimestamp = false;
- // Fill in the new address and attempt the update.
- obj.addmember(family).string(client_addr);
+ // Fill in the new address and attempt the update. We can do this using the "existing" output
+ // because we have the object janitor'd and the cache_update copies the object internally.
+ sessionData.addmember(family).string(client_addr);
try {
- if (!cache_update(request, key, version, obj)) {
- obj.destroy();
+ if (!cache_update(request, key, version, sessionData)) {
// This signals a version mismatch in which the original session we were asked to read
// has already been updated by another thread or process. In this case, we recurse the
// read attempt. This has to terminate eventually...right?
@@ -432,7 +433,7 @@ DDF FilesystemSessionCache::cache_read(
// Failure here is just ignored.
}
- return obj;
+ return sessionDataJanitor.release();
}
bool FilesystemSessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData)
@@ -586,7 +587,7 @@ void FilesystemSessionCache::store_session(SPRequest* request, const char* path,
if (!encrypted || !*encrypted) {
throw OperationException("No encrypted session data returned from sealer operation.");
}
-
+
ofstream os(path);
if (!os) {
log(ERROR_MARK, "error writing new session to file (%s), errno=%d", path, errno);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list