Skip to content

Commit f1642eb

Browse files
bmehta001Copilot
andauthored
Fix unsafe resource lifetimes found by audit (#1524)
Correct four independent ownership defects that can over-release borrowed CoreFoundation data, leak a newly created manager on registry allocation failure, corrupt self-assigned EventProperty values, or pair array allocations with scalar deletion. Files changed: - examples/cpp/MacProxy/main.cpp: preserve borrowed proxy values and guard empty proxy arrays. - lib/api/LogManagerFactory.cpp: retain manager ownership until registry insertion succeeds. - lib/system/EventProperty.cpp: make copy self-assignment safe. - lib/tracing/api/DebugProviders.hpp: pair new[] buffers with delete[]. - tests/unittests/EventPropertiesTests.cpp: cover EventProperty self-assignment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7d2f27a-7339-4585-ad02-9f89ce20ef40
1 parent 6571167 commit f1642eb

5 files changed

Lines changed: 22 additions & 13 deletions

File tree

examples/cpp/MacProxy/main.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ std::string GetProxyForURL(const std::string& url)
5858
urlProxArrayRef = CFNetworkCopyProxiesForURL(urlRef, proxyDicRef);
5959
if (!urlProxArrayRef)
6060
goto cleanup;
61+
if (CFArrayGetCount(urlProxArrayRef) == 0)
62+
goto cleanup;
6163

6264
defProxyDic = (CFDictionaryRef)CFArrayGetValueAtIndex(urlProxArrayRef, 0);
6365
if (!defProxyDic)
@@ -82,11 +84,6 @@ std::string GetProxyForURL(const std::string& url)
8284

8385
cleanup:
8486

85-
if (hostNameRef)
86-
{
87-
CFRelease(hostNameRef);
88-
hostNameRef = NULL;
89-
}
9087
if (urlProxArrayRef)
9188
{
9289
CFRelease(urlProxArrayRef);

lib/api/LogManagerFactory.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <cassert>
1414
#include <functional>
1515
#include <iostream>
16+
#include <memory>
1617
#include <utility>
1718

1819
#include <ctime>
@@ -33,9 +34,9 @@ namespace MAT_NS_BEGIN
3334
ILogManager* LogManagerFactory::Create(ILogConfiguration& configuration)
3435
{
3536
LOCKGUARD(ILogManagerInternal::managers_lock);
36-
auto logManager = new LogManagerImpl(configuration);
37-
ILogManagerInternal::managers.emplace(logManager);
38-
return logManager;
37+
auto logManager = std::make_unique<LogManagerImpl>(configuration);
38+
ILogManagerInternal::managers.emplace(logManager.get());
39+
return logManager.release();
3940
}
4041

4142
/// <summary>
@@ -254,4 +255,3 @@ namespace MAT_NS_BEGIN
254255

255256
}
256257
MAT_NS_END
257-

lib/system/EventProperty.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,11 @@ namespace MAT_NS_BEGIN {
504504
/// </summary>
505505
EventProperty& EventProperty::operator=(const EventProperty& source)
506506
{
507+
if (this == &source)
508+
{
509+
return *this;
510+
}
511+
507512
clear();
508513
memcpy((void*)this, (void*)&source, sizeof(EventProperty));
509514
copydata(&source);
@@ -958,4 +963,3 @@ namespace MAT_NS_BEGIN {
958963
}
959964

960965
} MAT_NS_END
961-

lib/tracing/api/DebugProviders.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,8 @@ class ETWStringStream : public DebugStringStream<ETWStringStreamBuffer> {
344344
guid.Data4[6] = buffer2[14];
345345
guid.Data4[7] = buffer2[15];
346346

347-
delete buffer;
348-
delete buffer2;
347+
delete[] buffer;
348+
delete[] buffer2;
349349

350350
return guid;
351351
}
@@ -393,4 +393,3 @@ class ETWStringStream : public DebugStringStream<ETWStringStreamBuffer> {
393393
#endif
394394

395395
#endif
396-

tests/unittests/EventPropertiesTests.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,15 @@ TEST(EventPropertiesTests, Properties)
114114
EXPECT_THAT(ep.GetPiiProperties(), IsEmpty());
115115
}
116116

117+
TEST(EventPropertiesTests, SelfAssignmentPreservesValue)
118+
{
119+
EventProperty property("value");
120+
121+
property = property;
122+
123+
EXPECT_EQ(property, EventProperty("value"));
124+
}
125+
117126
TEST(EventPropertiesTests, NumericProperties)
118127
{
119128
EventProperties ep("test");

0 commit comments

Comments
 (0)