From f9dfe1753c971dc6c3619167595587c123311c62 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 06:19:59 -0500 Subject: [PATCH 1/9] Bound telemetry inputs to prevent host stack and memory exhaustion Collect only argv[0] without recursive regex and cap SDK-collected metadata, HTTP response buffers, session files, and diagnostic decoder allocations. Preserve caller event-size policies and add boundary and small-stack regressions. Files changed: - docs/CsProtocol-decoding.md - docs/PAL.md - lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java - lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java - lib/bond/CompactBinaryProtocolReader.hpp - lib/decoder/PayloadDecoder.cpp - lib/http/HttpClient_Android.cpp - lib/http/HttpClient_Apple.mm - lib/http/HttpClient_Curl.hpp - lib/http/HttpClient_WinHttp.cpp - lib/http/HttpClient_WinInet.cpp - lib/http/HttpClient_WinRt.cpp - lib/include/public/IHttpClient.hpp - lib/offline/LogSessionDataProvider.cpp - lib/pal/PAL.cpp - lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - lib/pal/posix/DeviceInformationImpl_Android.cpp - lib/pal/posix/SystemInformationImpl_Android.cpp - lib/pal/posix/sysinfo_sources.cpp - lib/pal/posix/sysinfo_sources.hpp - lib/pal/posix/sysinfo_utils_apple.cpp - lib/pal/posix/sysinfo_utils_ios.mm - lib/pal/posix/sysinfo_utils_mac.mm - lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp - lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp - lib/shared/PlatformHelpers.h - lib/utils/FileUtils.cpp - lib/utils/FileUtils.hpp - lib/utils/Utils.cpp - lib/utils/Utils.hpp - lib/utils/ZlibUtils.cpp - lib/utils/ZlibUtils.hpp - tests/unittests/CMakeLists.txt - tests/unittests/HttpClientCurlTests.cpp - tests/unittests/HttpClientTests.cpp - tests/unittests/LogSessionDataTests.cpp - tests/unittests/PayloadDecoderTests.cpp - tests/unittests/ZlibUtilsTests.cpp - tests/unittests/bounded-input-tests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/CsProtocol-decoding.md | 18 ++ docs/PAL.md | 34 +++ .../applications/events/HttpClient.java | 91 ++++---- .../events/HttpClientRequestTest.java | 30 +++ lib/bond/CompactBinaryProtocolReader.hpp | 15 +- lib/decoder/PayloadDecoder.cpp | 15 +- lib/http/HttpClient_Android.cpp | 40 +++- lib/http/HttpClient_Apple.mm | 18 ++ lib/http/HttpClient_Curl.hpp | 49 +++-- lib/http/HttpClient_WinHttp.cpp | 9 +- lib/http/HttpClient_WinInet.cpp | 8 + lib/http/HttpClient_WinRt.cpp | 29 ++- lib/include/public/IHttpClient.hpp | 1 + lib/offline/LogSessionDataProvider.cpp | 5 + lib/pal/PAL.cpp | 28 +-- .../WindowsDesktopDeviceInformationImpl.cpp | 8 +- .../WindowsDesktopSystemInformationImpl.cpp | 26 ++- .../posix/DeviceInformationImpl_Android.cpp | 17 +- .../posix/SystemInformationImpl_Android.cpp | 16 +- lib/pal/posix/sysinfo_sources.cpp | 175 ++++++++++----- lib/pal/posix/sysinfo_sources.hpp | 23 +- lib/pal/posix/sysinfo_utils_apple.cpp | 5 +- lib/pal/posix/sysinfo_utils_ios.mm | 11 +- lib/pal/posix/sysinfo_utils_mac.mm | 12 +- .../WindowsRuntimeDeviceInformationImpl.cpp | 5 +- .../WindowsRuntimeSystemInformationImpl.cpp | 10 +- lib/shared/PlatformHelpers.h | 7 + lib/utils/FileUtils.cpp | 28 ++- lib/utils/FileUtils.hpp | 3 +- lib/utils/Utils.cpp | 46 ++++ lib/utils/Utils.hpp | 11 +- lib/utils/ZlibUtils.cpp | 30 ++- lib/utils/ZlibUtils.hpp | 2 + tests/unittests/CMakeLists.txt | 1 + tests/unittests/HttpClientCurlTests.cpp | 52 ++++- tests/unittests/HttpClientTests.cpp | 21 ++ tests/unittests/LogSessionDataTests.cpp | 8 + tests/unittests/PayloadDecoderTests.cpp | 17 ++ tests/unittests/ZlibUtilsTests.cpp | 38 ++++ tests/unittests/bounded-input-tests.cpp | 207 ++++++++++++++++++ 40 files changed, 943 insertions(+), 226 deletions(-) create mode 100644 tests/unittests/bounded-input-tests.cpp diff --git a/docs/CsProtocol-decoding.md b/docs/CsProtocol-decoding.md index c02901780..c06d9b2ac 100644 --- a/docs/CsProtocol-decoding.md +++ b/docs/CsProtocol-decoding.md @@ -28,3 +28,21 @@ Please refer to usage examples: * EventDecoderListener.cpp [EventDecoderListener::DecodeBuffer](../tests/functests/EventDecoderListener.cpp) for HTTP(s) request - Package of Records decoding * APITest.cpp [UTC_Callback_Test](../tests/functests/APITest.cpp) for Common Schema Record decoding + +## Decoder input limits + +Diagnostic request decoding rejects inputs larger than 64 MiB, including +compressed input and the inflated output. Inflation uses a fixed-size work +buffer and stops before growing the output beyond that limit; it does not +estimate output size from a presumed compression ratio. + +Binary container counts are limited to 65,536 elements and must fit the +remaining encoded input (at least one byte per sequence element or two bytes +per map entry). Invalid counts are rejected before generated readers allocate +their containers. Requests are also limited to 65,536 decoded records. +Decoding failure returns `false` rather than reporting a partial request as +success; partial inflation output +must not be used after a failed call. + +These diagnostic limits do not change the event submission API or its +configured serialized-event size policy. diff --git a/docs/PAL.md b/docs/PAL.md index 767e97794..8dbb438f7 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -59,6 +59,40 @@ ## Remote configuration (ECS client) +## Input size limits + +Automatically collected system, device, application, and network-provider +strings are limited to 4 KiB of UTF-8, without splitting a UTF-8 sequence. +This does not limit values explicitly supplied by an application through +event properties or semantic context. + +| Input | Limit | Oversize behavior | +| --- | --- | --- | +| POSIX application identifier | 4 KiB, stopping at the first NUL in `/proc/self/cmdline` | Truncate the executable name; never collect arguments or run a regex | +| POSIX OS release file and device-ID command output | 64 KiB | Bound the collected prefix and log a warning | +| OS-sized executable path and device-model buffers | 64 KiB | Reject before allocating | +| Windows version-resource and adapter-information buffers | 1 MiB | Reject before allocating; retain the existing missing-information fallback | +| Session sidecar file read through `FileGetContents` | 4 KiB | Reject the whole file, log a warning, and regenerate session data | +| HTTP response body | 16 MiB | Fail the request rather than retain an oversized response | +| HTTP response headers | 64 KiB | Fail the request rather than retain oversized headers | + +POSIX OS release values use exact line-key matching rather than recursive +regular expressions. Curl response headers are also parsed without regex. +The header budget includes framing for native raw headers or a minimum +four-byte allowance per name/value pair. Windows native queries measure raw +UTF-16/ANSI buffer bytes; WinRT conservatively budgets up to three UTF-8 bytes +per UTF-16 unit. Android bounds Java strings before JNI additionally checks +their encoded byte sizes. OS networking frameworks may have their own +internal limits; the SDK limits its own copies and streaming body reads. + +Command output is drained after the stored prefix reaches its limit so closing +the pipe cannot block behind a child waiting to write to a full pipe. + +Caller-provided events retain the existing configured serialized-event, +upload, and offline-cache size policies; no new per-property limit is applied. +See [decoder limits](CsProtocol-decoding.md#decoder-input-limits) for the +separate diagnostic decoding budget. + ## Bandwidth manager (Resource manager) - Get available bandwidth diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java index fb293e3ec..221edce2f 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java @@ -27,6 +27,9 @@ import androidx.annotation.RequiresApi; import java.io.BufferedInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.URL; @@ -87,6 +90,42 @@ public final void onCapabilitiesChanged( class Request implements HttpClientRequest { + static final int MAX_HTTP_RESPONSE_SIZE = 16 * 1024 * 1024; + static final int MAX_HTTP_RESPONSE_HEADERS_SIZE = 64 * 1024; + + static byte[] readResponseBody(InputStream input) throws IOException { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int count; + while ((count = input.read(buffer)) != -1) { + if (count > MAX_HTTP_RESPONSE_SIZE - output.size()) { + throw new IOException("HTTP response body exceeds buffered size limit"); + } + output.write(buffer, 0, count); + } + return output.toByteArray(); + } + + static String[] readResponseHeaders(Map> headers) throws IOException { + Vector result = new Vector<>(); + int size = 0; + for (Map.Entry> entry : headers.entrySet()) { + if (entry.getKey() != null) { + for (String value : entry.getValue()) { + // Pre-bound Java strings before JNI checks their encoded byte sizes. + long count = (long) entry.getKey().length() + value.length() + 4; + if (count > MAX_HTTP_RESPONSE_HEADERS_SIZE - size) { + throw new IOException("HTTP response headers exceed buffered size limit"); + } + size += (int) count; + result.add(entry.getKey()); + result.add(value); + } + } + } + return result.toArray(new String[0]); + } + Request( @NonNull HttpClient parent, String url, @@ -111,8 +150,6 @@ class Request implements HttpClientRequest { } public void run() { - final boolean logExceptions = false; - String[] headerArray = {}; byte[] body = {}; int response = 0; @@ -122,49 +159,19 @@ public void run() { body_stream.write(m_body); } response = m_connection.getResponseCode(); // may throw - Map> headers = m_connection.getHeaderFields(); - Vector headerList = new Vector<>(); - for (Map.Entry> entry : headers.entrySet()) { - if (entry.getKey() != null) { - for (String v : entry.getValue()) { - headerList.add(entry.getKey()); - headerList.add(v); - } - } - } - headerArray = headerList.toArray(headerArray); - BufferedInputStream in; - if (response >= 300) { - in = new BufferedInputStream(m_connection.getErrorStream()); - } else { - in = new BufferedInputStream(m_connection.getInputStream()); - } - byte[] buffer = new byte[1024]; - Vector buffers = new Vector<>(); - int size = 0; - while (true) { - int n = in.read(buffer, 0, 1024); - if (n < 0) { - break; - } - if (n > 0) { - buffers.add(java.util.Arrays.copyOfRange(buffer, 0, n)); - size += n; - } - } - body = new byte[size]; - int index = 0; - for (byte[] chunk : buffers) { - for (byte b : chunk) { - body[index] = b; - index += 1; + headerArray = readResponseHeaders(m_connection.getHeaderFields()); + InputStream stream = response >= 300 + ? m_connection.getErrorStream() : m_connection.getInputStream(); + if (stream != null) { + try (BufferedInputStream in = new BufferedInputStream(stream)) { + body = readResponseBody(in); } } } catch (Exception e) { - /* pass this on as a response of 0 */ - if (logExceptions) { - Log.e("MAE", "Exception in callback", e); - } + Log.e("MAE", "HTTP response failed", e); + response = -1; + headerArray = new String[0]; + body = new byte[0]; } finally { m_connection.disconnect(); } diff --git a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java index 523bcc9f3..4bca802f2 100644 --- a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java +++ b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java @@ -13,11 +13,41 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.Collections; +import java.io.ByteArrayInputStream; +import java.io.IOException; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.*; public class HttpClientRequestTest { + @Test + public void acceptsExactlyFullResponseBody() throws IOException { + byte[] body = new byte[Request.MAX_HTTP_RESPONSE_SIZE]; + Assert.assertArrayEquals(body, Request.readResponseBody(new ByteArrayInputStream(body))); + } + + @Test(expected = IOException.class) + public void rejectsOversizedResponseBody() throws IOException { + Request.readResponseBody(new ByteArrayInputStream(new byte[Request.MAX_HTTP_RESPONSE_SIZE + 1])); + } + + @Test + public void acceptsExactlyFullResponseHeaders() throws IOException { + String value = new String(new char[Request.MAX_HTTP_RESPONSE_HEADERS_SIZE - 5]).replace('\0', 'x'); + Map> headers = new HashMap<>(); + headers.put("X", Collections.singletonList(value)); + Assert.assertArrayEquals(new String[] {"X", value}, Request.readResponseHeaders(headers)); + } + + @Test(expected = IOException.class) + public void rejectsOversizedAggregateResponseHeaders() throws IOException { + String value = new String(new char[Request.MAX_HTTP_RESPONSE_HEADERS_SIZE]).replace('\0', 'x'); + Map> headers = new HashMap<>(); + headers.put("X", Collections.singletonList(value)); + Request.readResponseHeaders(headers); + } + @Test public void testHeaders() { byte[] bytes = "key1value1key33key2value2".getBytes(UTF_8); diff --git a/lib/bond/CompactBinaryProtocolReader.hpp b/lib/bond/CompactBinaryProtocolReader.hpp index 708aba421..a1502a8eb 100644 --- a/lib/bond/CompactBinaryProtocolReader.hpp +++ b/lib/bond/CompactBinaryProtocolReader.hpp @@ -26,6 +26,8 @@ class CompactBinaryProtocolReader { size_t m_ofs; public: + static constexpr uint32_t MAX_CONTAINER_ELEMENTS = 64 * 1024; + CompactBinaryProtocolReader(std::vector const& input) : m_input(input), m_ofs(0) @@ -186,6 +188,10 @@ class CompactBinaryProtocolReader { if (length > m_input.size() - m_ofs) { return false; } + if (length == 0) { + value.clear(); + return true; + } value.assign(reinterpret_cast(&m_input[m_ofs]), length); m_ofs += length; return true; @@ -214,14 +220,18 @@ class CompactBinaryProtocolReader { } elementType = (raw & 31); - return ReadUInt32(size); + return ReadUInt32(size) && + size <= MAX_CONTAINER_ELEMENTS && + size <= m_input.size() - m_ofs; } bool ReadMapContainerBegin(uint32_t& size, uint8_t& keyType, uint8_t& valueType) { return ReadUInt8(keyType) && (keyType >> 5 == 0) && ReadUInt8(valueType) && (valueType >> 5 == 0) && - ReadUInt32(size); + ReadUInt32(size) && + size <= MAX_CONTAINER_ELEMENTS && + size <= (m_input.size() - m_ofs) / 2; } bool ReadContainerEnd() @@ -277,4 +287,3 @@ class CompactBinaryProtocolReader { }; } // namespace bond_lite - diff --git a/lib/decoder/PayloadDecoder.cpp b/lib/decoder/PayloadDecoder.cpp index 6d86a7057..b974d03c8 100644 --- a/lib/decoder/PayloadDecoder.cpp +++ b/lib/decoder/PayloadDecoder.cpp @@ -71,6 +71,11 @@ namespace clienttelemetry { size_t length = 0; while (i < request.size()) { + if (v.size() >= bond_lite::CompactBinaryProtocolReader::MAX_CONTAINER_ELEMENTS) + { + TEST_LOG_ERROR("Too many records in decoder input"); + return {}; + } Record result; length = request.size() - i; std::vector test(request.data() + i, request.data() + i + length); @@ -97,7 +102,7 @@ namespace clienttelemetry { } if (!found) { - j += 1; + j = length + 1; } std::vector input(request.data() + i, request.data() + i + j - 1); bond_lite::CompactBinaryProtocolReader reader(input); @@ -105,12 +110,11 @@ namespace clienttelemetry { if (!Deserialize(reader, result, false)) { TEST_LOG_ERROR("Deserialization failed!"); - goto fail; + return {}; } i += j - 1; v.push_back(result); } - fail: return v; } @@ -542,6 +546,11 @@ namespace MAT_NS_BEGIN { bool DecodeRequest(const std::vector& in, std::string& out, bool compressed) { out.clear(); + if (in.size() > ZlibUtils::MAX_INFLATED_SIZE) + { + TEST_LOG_ERROR("Request exceeds decoder size limit"); + return false; + } std::vector buffer; if (compressed) diff --git a/lib/http/HttpClient_Android.cpp b/lib/http/HttpClient_Android.cpp index 9b93ea451..7b1af48bf 100644 --- a/lib/http/HttpClient_Android.cpp +++ b/lib/http/HttpClient_Android.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // #include "HttpClient_Android.hpp" +#include "pal/PAL.hpp" #include #include #include @@ -545,6 +546,8 @@ extern "C" JNIEXPORT void jobjectArray headers, jbyteArray body) { + using PAL::getMATSDKLogComponent; + size_t id_length = env->GetStringUTFLength(id); auto id_utf = env->GetStringUTFChars(id, nullptr); std::string id_string(id_utf, id_utf + id_length); @@ -560,23 +563,48 @@ extern "C" JNIEXPORT void auto response = new Microsoft::Applications::Events::HttpClient_Android::HttpResponse(request->GetId()); response->SetResponse(statusCode); + size_t headerBytes = 0; + bool oversized = false; size_t n_headers = env->GetArrayLength(headers); for (size_t i = 0; (i + 1u) < n_headers; i += 2) { auto k = static_cast(env->GetObjectArrayElement(headers, i)); auto v = static_cast(env->GetObjectArrayElement(headers, i + 1)); + const size_t keyBytes = env->GetStringUTFLength(k); + const size_t valueBytes = env->GetStringUTFLength(v); + if (4 > MAT::MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes || + keyBytes > MAT::MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 || + valueBytes > MAT::MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 - keyBytes) + { + LOG_WARN("HTTP response headers exceed %zu bytes; rejecting", MAT::MAX_HTTP_RESPONSE_HEADERS_SIZE); + env->DeleteLocalRef(k); + env->DeleteLocalRef(v); + oversized = true; + break; + } + headerBytes += keyBytes + valueBytes + 4; const char* k_utf = env->GetStringUTFChars(k, nullptr); - std::string key(k_utf, env->GetStringUTFLength(k)); + std::string key(k_utf, keyBytes); env->ReleaseStringUTFChars(k, k_utf); const char* v_utf = env->GetStringUTFChars(v, nullptr); - std::string value(v_utf, env->GetStringUTFLength(v)); + std::string value(v_utf, valueBytes); env->ReleaseStringUTFChars(v, v_utf); + env->DeleteLocalRef(k); + env->DeleteLocalRef(v); response->AddHeader(std::move(key), std::move(value)); } - auto body_pointer = env->GetByteArrayElements(body, nullptr); - response->SetBody(env->GetArrayLength(body), - reinterpret_cast(body_pointer)); - env->ReleaseByteArrayElements(body, body_pointer, JNI_ABORT); + const size_t bodyBytes = env->GetArrayLength(body); + if (oversized || bodyBytes > MAT::MAX_HTTP_RESPONSE_SIZE) + { + LOG_WARN("HTTP response exceeds buffered size limits; rejecting"); + response->SetResult(MAT::HttpResult_NetworkFailure); + } + else if (bodyBytes != 0) + { + auto body_pointer = env->GetByteArrayElements(body, nullptr); + response->SetBody(bodyBytes, reinterpret_cast(body_pointer)); + env->ReleaseByteArrayElements(body, body_pointer, JNI_ABORT); + } // callback will own response callback->OnHttpResponse(response); } diff --git a/lib/http/HttpClient_Apple.mm b/lib/http/HttpClient_Apple.mm index 92b0ac2e8..b13d92157 100644 --- a/lib/http/HttpClient_Apple.mm +++ b/lib/http/HttpClient_Apple.mm @@ -350,11 +350,25 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) simpleResponse->m_statusCode = (httpResp != nil) ? static_cast(httpResp.statusCode) : 0; + bool headersTooLarge = false; + size_t headerBytes = 0; if (httpResp != nil) { NSDictionary *responseHeaders = [httpResp allHeaderFields]; for (id key in responseHeaders) { + const NSUInteger keyBytes = [key lengthOfBytesUsingEncoding:NSUTF8StringEncoding]; + const NSUInteger valueBytes = [responseHeaders[key] lengthOfBytesUsingEncoding:NSUTF8StringEncoding]; + if (4 > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes || + keyBytes > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 || + valueBytes > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 - keyBytes) + { + LOG_WARN("HTTP response headers exceed %zu bytes; rejecting", MAX_HTTP_RESPONSE_HEADERS_SIZE); + headersTooLarge = true; + simpleResponse->m_headers.clear(); + break; + } + headerBytes += keyBytes + valueBytes + 4; const char* keyString = [key UTF8String]; const char* valueString = [responseHeaders[key] UTF8String]; if (keyString != nullptr && valueString != nullptr) @@ -368,6 +382,10 @@ void HandleResponse(NSData* data, NSURLResponse* response, NSError* error) { simpleResponse->m_result = HttpResult_Aborted; } + else if (headersTooLarge) + { + simpleResponse->m_result = HttpResult_NetworkFailure; + } else if (error) { NSString* errorDomain = [error domain]; diff --git a/lib/http/HttpClient_Curl.hpp b/lib/http/HttpClient_Curl.hpp index ff9bef44b..2fa6e592b 100644 --- a/lib/http/HttpClient_Curl.hpp +++ b/lib/http/HttpClient_Curl.hpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include @@ -45,8 +44,6 @@ #endif #define HTTP_CONN_TIMEOUT 5L -#define HTTP_STATUS_REGEXP "HTTP\\/\\d\\.\\d (\\d+)\\ .*" -#define HTTP_HEADER_REGEXP "(.*)\\: (.*)\\n*" #undef TRACE #define TRACE(...) // printf @@ -476,6 +473,8 @@ class CurlHttpOperation { if (rawResponse) { if (!SetOption(CURLOPT_HEADER, 1L) || + !SetOption(CURLOPT_HEADERFUNCTION, &WriteHeaderCallback) || + !SetOption(CURLOPT_HEADERDATA, static_cast(&respHeaders)) || !SetOption(CURLOPT_WRITEFUNCTION, &WriteMemoryCallback) || !SetOption(CURLOPT_WRITEDATA, static_cast(&response))) { @@ -483,7 +482,7 @@ class CurlHttpOperation { goto cleanup; } } else { - if (!SetOption(CURLOPT_HEADERFUNCTION, &WriteVectorCallback) || + if (!SetOption(CURLOPT_HEADERFUNCTION, &WriteHeaderCallback) || !SetOption(CURLOPT_HEADERDATA, static_cast(&respHeaders)) || !SetOption(CURLOPT_WRITEFUNCTION, &WriteVectorCallback) || !SetOption(CURLOPT_WRITEDATA, static_cast(&respBody))) @@ -530,17 +529,6 @@ class CurlHttpOperation { goto cleanup; } - /* Code snippet to parse raw HTTP response. This might come in handy - * if we ever consider to handle the raw upload instead of curl_easy_perform - ... - std::string resp((const char *)response); - std::regex http_status_regex(HTTP_STATUS_REGEXP); - std::smatch match; - if(std::regex_search(resp, match, http_status_regex)) - http_code = std::stol(match[1]); - ... - */ - /* libcurl is nice enough to parse the response code itself: */ infoResult = curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &httpStatusCode); if (infoResult != CURLE_OK) @@ -671,10 +659,14 @@ class CurlHttpOperation { std::string header; while (std::getline(ss, header, '\n')) { - std::smatch match; - std::regex http_headers_regex(HTTP_HEADER_REGEXP); - if (std::regex_search(header, match, http_headers_regex)) - result[match[1]] = match[2]; // Key: value + if (!header.empty() && header.back() == '\r') + header.pop_back(); + const size_t colon = header.find(':'); + if (colon == std::string::npos || colon == 0) + continue; + const size_t value = header.find_first_not_of(" \t", colon + 1); + result[header.substr(0, colon)] = value == std::string::npos + ? std::string() : header.substr(value); } return result; } @@ -976,7 +968,7 @@ class CurlHttpOperation { // stops a hostile or MITM'd collector from driving unbounded memory growth by // returning an oversized body (a memory-amplification DoS of the embedding process). // Exceeding it aborts the transfer, so the upload is treated as failed and retried. - static constexpr size_t kMaxResponseBytes = 16 * 1024 * 1024; // 16 MB + static constexpr size_t kMaxResponseBytes = MAX_HTTP_RESPONSE_SIZE; // Raw response buffer struct MemoryStruct { @@ -1038,6 +1030,16 @@ class CurlHttpOperation { * @return */ static size_t WriteVectorCallback(char* ptr, size_t size, size_t nmemb, void* userp) + { + return WriteBoundedVector(ptr, size, nmemb, userp, MAX_HTTP_RESPONSE_SIZE); + } + + static size_t WriteHeaderCallback(char* ptr, size_t size, size_t nmemb, void* userp) + { + return WriteBoundedVector(ptr, size, nmemb, userp, MAX_HTTP_RESPONSE_HEADERS_SIZE); + } + + static size_t WriteBoundedVector(char* ptr, size_t size, size_t nmemb, void* userp, size_t limit) { // Guard the size * nmemb product against size_t overflow before using it. if (nmemb != 0 && size > static_cast(-1) / nmemb) { @@ -1046,11 +1048,10 @@ class CurlHttpOperation { size_t realsize = size * nmemb; auto* data = static_cast*>(userp); if (data != nullptr) { - // SECURITY: bound the buffered response (see kMaxResponseBytes). Compare - // overflow-safely (data->size() is always <= kMaxResponseBytes here). + // Bound headers and bodies with their respective budgets. // Returning a short count aborts the transfer with CURLE_WRITE_ERROR. - if (realsize > kMaxResponseBytes - data->size()) { - TRACE("Response exceeds max buffered size (%zu bytes); aborting transfer\n", kMaxResponseBytes); + if (data->size() > limit || realsize > limit - data->size()) { + LOG_WARN("HTTP response exceeds %zu buffered bytes; aborting transfer", limit); return 0; } const auto* begin = reinterpret_cast(ptr); diff --git a/lib/http/HttpClient_WinHttp.cpp b/lib/http/HttpClient_WinHttp.cpp index 9230f9277..10259e2c9 100644 --- a/lib/http/HttpClient_WinHttp.cpp +++ b/lib/http/HttpClient_WinHttp.cpp @@ -1309,7 +1309,13 @@ class WinHttpRequestWrapper : public std::enable_shared_from_this 0) { - if (headerBytes % sizeof(wchar_t) != 0) + if (headerBytes > MAX_HTTP_RESPONSE_HEADERS_SIZE) + { + LOG_WARN("HTTP response headers exceed %zu bytes; rejecting", MAX_HTTP_RESPONSE_HEADERS_SIZE); + response->m_result = HttpResult_NetworkFailure; + response->m_body.clear(); + } + else if (headerBytes % sizeof(wchar_t) != 0) { LOG_WARN("WinHttpQueryHeaders(RAW_HEADERS_CRLF) returned an invalid byte count: %lu", headerBytes); } @@ -1368,6 +1374,7 @@ class WinHttpRequestWrapper : public std::enable_shared_from_thism_result = HttpResult_NetworkFailure; break; diff --git a/lib/http/HttpClient_WinInet.cpp b/lib/http/HttpClient_WinInet.cpp index 06d8a2467..a57433aed 100644 --- a/lib/http/HttpClient_WinInet.cpp +++ b/lib/http/HttpClient_WinInet.cpp @@ -1305,6 +1305,14 @@ class WinInetRequestWrapper : public std::enable_shared_from_this MAX_HTTP_RESPONSE_HEADERS_SIZE) + { + LOG_WARN("HTTP response headers exceed %zu bytes; rejecting", MAX_HTTP_RESPONSE_HEADERS_SIZE); + response->m_result = HttpResult_NetworkFailure; + response->m_body.clear(); + } + else if (!headersQueried && headerError == ERROR_INSUFFICIENT_BUFFER && headerBytes > 0 && headerBytes < std::numeric_limits::max()) diff --git a/lib/http/HttpClient_WinRt.cpp b/lib/http/HttpClient_WinRt.cpp index c689ecd8a..1ebc965a2 100644 --- a/lib/http/HttpClient_WinRt.cpp +++ b/lib/http/HttpClient_WinRt.cpp @@ -187,6 +187,26 @@ namespace MAT_NS_BEGIN { response->m_result = HttpResult_OK; IMapView^ mapView = httpResponse->Headers->GetView(); + size_t headerBytes = 0; + auto addHeader = [&](String^ key, String^ value) + { + // Bound UTF-16 inputs before conversion; UTF-8 needs at most + // three bytes per UTF-16 code unit. + const uint64_t keyBytes = static_cast(key->Length()) * 3; + const uint64_t valueBytes = static_cast(value->Length()) * 3; + if (4 > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes || + keyBytes > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 || + valueBytes > MAX_HTTP_RESPONSE_HEADERS_SIZE - headerBytes - 4 - keyBytes) + { + LOG_WARN("HTTP response headers exceed %zu bytes; rejecting", MAX_HTTP_RESPONSE_HEADERS_SIZE); + response->m_result = HttpResult_NetworkFailure; + response->m_headers.clear(); + return false; + } + headerBytes += static_cast(keyBytes + valueBytes) + 4; + response->m_headers.add(from_platform_string(key), from_platform_string(value)); + return true; + }; auto iterator = mapView->First(); unsigned int index = 0; while (index < mapView->Size) @@ -194,7 +214,8 @@ namespace MAT_NS_BEGIN { String^ Key = iterator->Current->Key; String^ Value = iterator->Current->Value; - response->m_headers.add(from_platform_string(Key), from_platform_string(Value)); + if (!addHeader(Key, Value)) + break; iterator->MoveNext(); index++; } @@ -203,12 +224,13 @@ namespace MAT_NS_BEGIN { IMapView^ contentHeadersView = m_httpResponseMessage->Content->Headers->GetView(); auto contentHeadersiterator = contentHeadersView->First(); unsigned int contentHeadersIndex = 0; - while (contentHeadersIndex < contentHeadersView->Size) + while (response->m_result == HttpResult_OK && contentHeadersIndex < contentHeadersView->Size) { String^ Key = contentHeadersiterator->Current->Key; String^ Value = contentHeadersiterator->Current->Value; - response->m_headers.add(from_platform_string(Key), from_platform_string(Value)); + if (!addHeader(Key, Value)) + break; contentHeadersiterator->MoveNext(); contentHeadersIndex++; } @@ -222,6 +244,7 @@ namespace MAT_NS_BEGIN { try { IInputStream^ inputStream = nullptr; + if (response->m_result == HttpResult_OK) { auto streamOp = m_httpResponseMessage->Content->ReadAsInputStreamAsync(); auto streamTask = create_task(streamOp, m_cancellationTokenSource.get_token()); diff --git a/lib/include/public/IHttpClient.hpp b/lib/include/public/IHttpClient.hpp index 29b57edc0..0bd38b09b 100644 --- a/lib/include/public/IHttpClient.hpp +++ b/lib/include/public/IHttpClient.hpp @@ -31,6 +31,7 @@ namespace MAT_NS_BEGIN /// response and reports the request as a network failure so it is retried. /// static constexpr std::size_t MAX_HTTP_RESPONSE_SIZE = 16u * 1024u * 1024u; // 16 MB + static constexpr std::size_t MAX_HTTP_RESPONSE_HEADERS_SIZE = 64u * 1024u; /// /// The HttpHeaders class contains a set of HTTP headers. diff --git a/lib/offline/LogSessionDataProvider.cpp b/lib/offline/LogSessionDataProvider.cpp index ea3112c72..d15ab3caf 100644 --- a/lib/offline/LogSessionDataProvider.cpp +++ b/lib/offline/LogSessionDataProvider.cpp @@ -142,6 +142,11 @@ namespace MAT_NS_BEGIN uint64_t& sessionFirstTimeLaunch, std::string& sessionSDKUid) { + if (content.size() > MAX_FILE_CONTENTS_SIZE) + { + LOG_WARN("Session data exceeds %zu bytes; rejecting", MAX_FILE_CONTENTS_SIZE); + return false; + } if (content.empty()) { return false; } diff --git a/lib/pal/PAL.cpp b/lib/pal/PAL.cpp index c9d750232..ad471b0ce 100644 --- a/lib/pal/PAL.cpp +++ b/lib/pal/PAL.cpp @@ -568,35 +568,35 @@ namespace PAL_NS_BEGIN { { if (m_DeviceInformation != nullptr) { - context->SetDeviceId(m_DeviceInformation->GetDeviceId()); - context->SetDeviceModel(m_DeviceInformation->GetModel()); - context->SetDeviceMake(m_DeviceInformation->GetManufacturer()); + context->SetDeviceId(MAT::boundedSystemInfo(m_DeviceInformation->GetDeviceId().c_str())); + context->SetDeviceModel(MAT::boundedSystemInfo(m_DeviceInformation->GetModel().c_str())); + context->SetDeviceMake(MAT::boundedSystemInfo(m_DeviceInformation->GetManufacturer().c_str())); } if (m_SystemInformation != nullptr) { // Get SystemInfo common fields - context->SetOsVersion(m_SystemInformation->GetOsMajorVersion()); - context->SetOsName(m_SystemInformation->GetOsName()); - context->SetOsBuild(m_SystemInformation->GetOsFullVersion()); - context->SetDeviceClass(m_SystemInformation->GetDeviceClass()); + context->SetOsVersion(MAT::boundedSystemInfo(m_SystemInformation->GetOsMajorVersion().c_str())); + context->SetOsName(MAT::boundedSystemInfo(m_SystemInformation->GetOsName().c_str())); + context->SetOsBuild(MAT::boundedSystemInfo(m_SystemInformation->GetOsFullVersion().c_str())); + context->SetDeviceClass(MAT::boundedSystemInfo(m_SystemInformation->GetDeviceClass().c_str())); // AppInfo fields - context->SetAppId(m_SystemInformation->GetAppId()); - context->SetAppVersion(m_SystemInformation->GetAppVersion()); - context->SetAppLanguage(m_SystemInformation->GetAppLanguage()); + context->SetAppId(MAT::boundedSystemInfo(m_SystemInformation->GetAppId().c_str())); + context->SetAppVersion(MAT::boundedSystemInfo(m_SystemInformation->GetAppVersion().c_str())); + context->SetAppLanguage(MAT::boundedSystemInfo(m_SystemInformation->GetAppLanguage().c_str())); // UserInfo fields. - context->SetUserLanguage(m_SystemInformation->GetUserLanguage()); - context->SetUserTimeZone(m_SystemInformation->GetUserTimeZone()); + context->SetUserLanguage(MAT::boundedSystemInfo(m_SystemInformation->GetUserLanguage().c_str())); + context->SetUserTimeZone(MAT::boundedSystemInfo(m_SystemInformation->GetUserTimeZone().c_str())); //context->SetUserAdvertisingId(m_SystemInformation->GetUserAdvertisingId()); - context->SetCommercialId(m_SystemInformation->GetCommercialId()); + context->SetCommercialId(MAT::boundedSystemInfo(m_SystemInformation->GetCommercialId().c_str())); } if (m_NetworkInformation != nullptr) { // Get NetworkInfo common fields - context->SetNetworkProvider(m_NetworkInformation->GetNetworkProvider()); + context->SetNetworkProvider(MAT::boundedSystemInfo(m_NetworkInformation->GetNetworkProvider().c_str())); context->SetNetworkCost(m_NetworkInformation->GetNetworkCost()); context->SetNetworkType(m_NetworkInformation->GetNetworkType()); } diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index 3c8fe6baf..0cc145092 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -9,6 +9,7 @@ #include "pal/PAL.hpp" #include "pal/DeviceInformationImpl.hpp" #include "utils/StringUtils.hpp" +#include "utils/Utils.hpp" #include "WindowsEnvironmentInfo.hpp" MATSDK_LOG_INST_COMPONENT_NS("DeviceInfo", "Win32 Desktop Device Information") @@ -61,11 +62,16 @@ namespace PAL_NS_BEGIN { * the ulOutBufLen parameter contains the required buffer size, so we retry with * suggested value of ulOutBufLen, assuming it is bigger than the default. */ - if (ulOutBufLen > sizeof(IP_ADAPTER_INFO)) + if (ulOutBufLen > sizeof(IP_ADAPTER_INFO) && + ulOutBufLen <= MAX_SYSTEM_INFO_BLOB_SIZE) { FREE(pAdapterInfo); goto retry_bigger_buffer; } + if (ulOutBufLen > MAX_SYSTEM_INFO_BLOB_SIZE) + { + LOG_WARN("Adapter information exceeds %zu bytes; rejecting", MAX_SYSTEM_INFO_BLOB_SIZE); + } } if ((result == ERROR_SUCCESS) && (pAdapterInfo->AdapterName[0] != 0)) { diff --git a/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp index 8bc6b1147..d5f62651e 100644 --- a/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp @@ -120,6 +120,11 @@ namespace PAL_NS_BEGIN { { return{}; } + if (dwVersionInfoSize > MAX_SYSTEM_INFO_BLOB_SIZE) + { + LOG_WARN("Executable version resource exceeds %zu bytes; rejecting", MAX_SYSTEM_INFO_BLOB_SIZE); + return {}; + } buffer.resize(dwVersionInfoSize); @@ -190,10 +195,17 @@ namespace PAL_NS_BEGIN { const PCSTR c_dataCollection_Key = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\DataCollection"; const PCSTR c_commercialId = "CommercialId"; DWORD size = sizeof(buff); - if (RegGetValueA(HKEY_LOCAL_MACHINE, c_groupPolicyDataCollection_Key, c_commercialId, RRF_RT_REG_SZ, NULL, static_cast(buff), &size) != ERROR_SUCCESS) + LSTATUS result = RegGetValueA(HKEY_LOCAL_MACHINE, c_groupPolicyDataCollection_Key, c_commercialId, RRF_RT_REG_SZ, NULL, static_cast(buff), &size); + if (result != ERROR_SUCCESS) { size = sizeof(buff); - RegGetValueA(HKEY_LOCAL_MACHINE, c_dataCollection_Key, c_commercialId, RRF_RT_REG_SZ, NULL, static_cast(buff), &size); + result = RegGetValueA(HKEY_LOCAL_MACHINE, c_dataCollection_Key, c_commercialId, RRF_RT_REG_SZ, NULL, static_cast(buff), &size); + } + if (result != ERROR_SUCCESS) + { + if (result == ERROR_MORE_DATA) + LOG_WARN("Commercial identifier exceeds system information buffer; rejecting"); + return {}; } return buff; } @@ -242,9 +254,10 @@ namespace PAL_NS_BEGIN { HMODULE handle = nullptr; if (::GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, nullptr, &handle) != FALSE) { - if (::GetModuleFileNameA(handle, &buff[0], MAX_PATH) > 0) + DWORD length = ::GetModuleFileNameA(handle, &buff[0], MAX_PATH); + if (length > 0 && length < MAX_PATH) { - std::string app_name(buff); + std::string app_name(buff, length); size_t pos_dot = app_name.rfind("."); size_t pos_slash = app_name.rfind("\\"); if ((pos_dot != std::string::npos) && (pos_slash != std::string::npos) && (pos_dot > pos_slash)) @@ -253,6 +266,10 @@ namespace PAL_NS_BEGIN { } appId = app_name; } + else if (length == MAX_PATH) + { + LOG_WARN("Executable name exceeds application information buffer; rejecting"); + } } else { @@ -315,4 +332,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/pal/posix/DeviceInformationImpl_Android.cpp b/lib/pal/posix/DeviceInformationImpl_Android.cpp index fc132629d..991c6fa3e 100644 --- a/lib/pal/posix/DeviceInformationImpl_Android.cpp +++ b/lib/pal/posix/DeviceInformationImpl_Android.cpp @@ -5,6 +5,7 @@ #define LOG_MODULE DBG_API #include "pal/PAL.hpp" #include "pal/DeviceInformationImpl.hpp" +#include "utils/Utils.hpp" #include #include @@ -198,15 +199,16 @@ namespace PAL_NS_BEGIN { jStr = pEnv->GetStringUTFChars(androidIdJstr, &isCopy); std::string deviceId("a:"); - deviceId += jStr; + deviceId += MAT::boundedSystemInfo(jStr); + deviceId = MAT::boundedSystemInfo(deviceId.c_str()); pEnv->ReleaseStringUTFChars(androidIdJstr, jStr); jStr = pEnv->GetStringUTFChars(manufacturerJstr, &isCopy); - std::string manufacturer = jStr; + std::string manufacturer = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(manufacturerJstr, jStr); jStr = pEnv->GetStringUTFChars(modelJstr, &isCopy); - std::string model = jStr; + std::string model = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(modelJstr, jStr); AndroidDeviceInformationConnector::setDeviceId(std::move(deviceId)); @@ -248,17 +250,14 @@ Java_com_microsoft_applications_events_HttpClient_onPowerChange(JNIEnv* env, ) { auto start = env->GetStringUTFChars(id, nullptr); - auto end = start + env->GetStringUTFLength(id); - PAL::AndroidDeviceInformationConnector::setDeviceId(std::string(start, end)); + PAL::AndroidDeviceInformationConnector::setDeviceId(MAT::boundedSystemInfo(start)); env->ReleaseStringUTFChars(id, start); start = env->GetStringUTFChars(manufacturer, nullptr); - end = start + env->GetStringUTFLength(manufacturer); - PAL::AndroidDeviceInformationConnector::setManufacturer(std::string(start, end)); + PAL::AndroidDeviceInformationConnector::setManufacturer(MAT::boundedSystemInfo(start)); env->ReleaseStringUTFChars(manufacturer, start); start = env->GetStringUTFChars(model, nullptr); - end = start + env->GetStringUTFLength(model); - PAL::AndroidDeviceInformationConnector::setModel(std::string(start, end)); + PAL::AndroidDeviceInformationConnector::setModel(MAT::boundedSystemInfo(start)); env->ReleaseStringUTFChars(model, start); } diff --git a/lib/pal/posix/SystemInformationImpl_Android.cpp b/lib/pal/posix/SystemInformationImpl_Android.cpp index ed62bd259..25687ea9c 100644 --- a/lib/pal/posix/SystemInformationImpl_Android.cpp +++ b/lib/pal/posix/SystemInformationImpl_Android.cpp @@ -6,6 +6,7 @@ #include #include "pal/PAL.hpp" #include "pal/SystemInformationImpl.hpp" +#include "utils/Utils.hpp" #include @@ -28,8 +29,7 @@ namespace PAL_NS_BEGIN { static void setValue(JNIEnv *env, std::string & s, jstring js) { const char *start = env->GetStringUTFChars(js, nullptr); - auto end = start + env->GetStringUTFLength(js); - s = std::string(start, end); + s = MAT::boundedSystemInfo(start); env->ReleaseStringUTFChars(js, start); } @@ -102,7 +102,7 @@ namespace PAL_NS_BEGIN { jstring packageNameJstr = reinterpret_cast(pEnv->CallObjectMethod(activity, getPackageNameMid)); jStr = pEnv->GetStringUTFChars(packageNameJstr, &isCopy); - std::string packageName = jStr; + std::string packageName = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(packageNameJstr, jStr); std::string versionName; @@ -114,7 +114,7 @@ namespace PAL_NS_BEGIN { pEnv->GetObjectField(packageInfo, versionNameFid)); jStr = pEnv->GetStringUTFChars(versionNameJstr, &isCopy); - versionName = jStr; + versionName = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(versionNameJstr, jStr); } pEnv->ExceptionClear(); @@ -122,7 +122,7 @@ namespace PAL_NS_BEGIN { jobject defaultLocale = pEnv->CallStaticObjectMethod(localeClass, getDefaultLocaleMid); jstring launguageTagJstr = reinterpret_cast(pEnv->CallObjectMethod(defaultLocale, toLanguageTagMid)); jStr = pEnv->GetStringUTFChars(launguageTagJstr, &isCopy); - std::string languageTag = jStr; + std::string languageTag = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(launguageTagJstr, jStr); if (SDK_INT < 21) { @@ -131,19 +131,19 @@ namespace PAL_NS_BEGIN { jstring versionReleaseJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildVersionClass, releaseFid)); jStr = pEnv->GetStringUTFChars(versionReleaseJstr, &isCopy); - std::string versionRelease = jStr; + std::string versionRelease = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(versionReleaseJstr, jStr); jstring versionIncrementalJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildVersionClass, incrementalFid)); jStr = pEnv->GetStringUTFChars(versionIncrementalJstr, &isCopy); - std::string versionIncremental = jStr; + std::string versionIncremental = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(versionIncrementalJstr, jStr); std::string osVersion = versionRelease + " " + versionIncremental; AndroidSystemInformationConnector::s_app_id = std::move(packageName); AndroidSystemInformationConnector::s_app_language = std::move(languageTag); AndroidSystemInformationConnector::s_app_version = std::move(versionName); - AndroidSystemInformationConnector::s_os_full_version = std::move(osVersion); + AndroidSystemInformationConnector::s_os_full_version = MAT::boundedSystemInfo(osVersion.c_str()); AndroidSystemInformationConnector::s_os_major_version = std::move(versionRelease); AndroidSystemInformationConnector::s_device_class = (std::move(device_class)); } diff --git a/lib/pal/posix/sysinfo_sources.cpp b/lib/pal/posix/sysinfo_sources.cpp index 28f1e3552..a3c2d9c8b 100644 --- a/lib/pal/posix/sysinfo_sources.cpp +++ b/lib/pal/posix/sysinfo_sources.cpp @@ -10,6 +10,8 @@ */ #include "sysinfo_sources_impl.hpp" +#include "utils/Utils.hpp" +#include "pal/PAL.hpp" #include #include @@ -25,8 +27,6 @@ #include #include -#include - #include #include @@ -73,12 +73,22 @@ void get_platform_uuid(char * buf, int bufSize) std::string get_app_name() { + using PAL::getMATSDKLogComponent; std::vector appId(PATH_MAX+1, 0); - uint32_t length = 0; + uint32_t length = static_cast(appId.size()); if(_NSGetExecutablePath(&appId[0], &length)) { + if (length == 0 || length > MAT::MAX_SYSTEM_INFO_SOURCE_SIZE) + { + LOG_WARN("Executable path exceeds system information source limit"); + return {}; + } appId.resize(length, 0); - _NSGetExecutablePath(&appId[0], &length); + if (_NSGetExecutablePath(&appId[0], &length) != 0) + { + LOG_WARN("Unable to read executable path"); + return {}; + } } std::string result = basename(appId.data()); return result; @@ -91,11 +101,40 @@ std::string get_app_name() * @param filename * @return */ -inline std::string ReadFile(const char *filename) +inline std::string ReadFile(const char *filename, sysinfo_selector selector) { - std::ifstream t(filename); - std::string str((std::istreambuf_iterator(t)), std::istreambuf_iterator()); - return str; + using PAL::getMATSDKLogComponent; + std::ifstream input(filename, std::ios::binary); + std::string result; + const size_t limit = selector == sysinfo_selector::key_value + ? MAT::MAX_SYSTEM_INFO_SOURCE_SIZE : MAT::MAX_SYSTEM_INFO_VALUE_SIZE; + char ch; + while (result.size() < limit && input.get(ch)) + { + if ((selector == sysinfo_selector::first_null && ch == '\0') || + (selector == sysinfo_selector::first_line && ch == '\n')) + { + return result; + } + result.push_back(ch); + } + if (input.get(ch)) + { + if ((selector == sysinfo_selector::first_null && ch == '\0') || + (selector == sysinfo_selector::first_line && ch == '\n')) + { + return result; + } + LOG_WARN("System information source exceeds %zu bytes; truncating", limit); + // The extra byte lets boundedSystemInfo preserve UTF-8 boundaries. + result.push_back(ch); + } + if (input.bad()) + { + LOG_WARN("Unable to read system information source"); + return {}; + } + return result; } /** @@ -112,20 +151,35 @@ inline std::string ReadFile(const char *filename) #endif static std::string Exec(const char* cmd) { + using PAL::getMATSDKLogComponent; std::array buffer; std::string result; auto close_pipe = [](FILE* file) { pclose(file); }; std::unique_ptr pipe(popen(cmd, "r"), close_pipe); if (!pipe) { - // throw std::runtime_error("popen() failed!"); + LOG_WARN("Unable to execute system information command"); return result; } - while (!feof(pipe.get())) + bool truncated = false; + while (fgets(buffer.data(), static_cast(buffer.size()), pipe.get()) != nullptr) { - if (fgets(buffer.data(), buffer.size(), pipe.get())!=NULL) - result += buffer.data(); + const size_t count = strlen(buffer.data()); + const size_t remaining = MAT::MAX_SYSTEM_INFO_SOURCE_SIZE - result.size(); + result.append(buffer.data(), std::min(count, remaining)); + truncated = truncated || count > remaining; + // Drain the pipe even after reaching the cap so pclose cannot deadlock + // waiting for a child blocked on a full stdout pipe. + } + if (ferror(pipe.get())) + { + LOG_WARN("Unable to read system information command output"); + return {}; + } + if (truncated) + { + LOG_WARN("System information command output exceeds %zu bytes; truncating", MAT::MAX_SYSTEM_INFO_SOURCE_SIZE); } // Remove EOL. In all use-cases below we don't need it. @@ -143,37 +197,51 @@ static std::string Exec(const char* cmd) #endif /** - * Read node value, preprocess it using regexp and store result in cache + * Read a bounded node value, select it without regex and store it in cache * * @param key Field name * @return true if field value is found and saved in cache */ bool sysinfo_sources::fetch(std::string key) { - for(auto &kv : (*this)) - { - if(kv.first == key) - { - const std::string star("*"); - const std::string empty(""); - - std::string contents = ReadFile(kv.second.path); - if((kv.second.selector == star) || (kv.second.selector == empty)) - { - cache[key] = contents; - return true; - } - // Run regexp - std::regex selector_regex(kv.second.selector); - std::smatch match; - if(std::regex_search(contents, match, selector_regex)) - { - cache[key] = match[1]; - return true; - } - } - } - return false; + using PAL::getMATSDKLogComponent; + + for (auto& kv : (*this)) + { + if (kv.first != key) + continue; + if (kv.second.selector == sysinfo_selector::key_value && kv.second.name == nullptr) + { + LOG_WARN("System information key-value selector is missing its name"); + continue; + } + std::string contents = ReadFile(kv.second.path, kv.second.selector); + if (kv.second.selector != sysinfo_selector::key_value) + { + cache[key] = MAT::boundedSystemInfo(contents.c_str()); + return true; + } + std::istringstream lines(contents); + std::string line; + const std::string prefix = std::string(kv.second.name) + "="; + while (std::getline(lines, line)) + { + if (line.compare(0, prefix.size(), prefix) != 0) + continue; + std::string value = line.substr(prefix.size()); + if (!value.empty() && value.back() == '\r') + value.pop_back(); + if (value.size() >= 2 && + ((value.front() == '"' && value.back() == '"') || + (value.front() == '\'' && value.back() == '\''))) + { + value = value.substr(1, value.size() - 2); + } + cache[key] = MAT::boundedSystemInfo(value.c_str()); + return true; + } + } + return false; } /** @@ -206,6 +274,8 @@ const std::string& sysinfo_sources::get(std::string key) { if(cache.find(key) == cache.end()) fetch(key); + if (cache[key].size() > MAT::MAX_SYSTEM_INFO_VALUE_SIZE) + cache[key] = MAT::boundedSystemInfo(cache[key].c_str()); return cache[key]; } @@ -218,24 +288,11 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() uname(&buf); #if defined(__linux__) // Obtain Linux system information from filesystem - add("devId", { "/etc/machine-id", "*"}); - add("osName", {"/etc/os-release", ".*ID=(.*)[\n]+"}); - add("osVer", {"/etc/os-release", ".*VERSION_ID=\"(.*)\".*"}); - add("osRel", {"/etc/os-release", ".*VERSION=\"(.*)\".*"}); - add("osBuild", {"/proc/version", "(.*)[\n]+"}); - // add("proc_loadavg", {"/proc/loadavg", "(.*)[\n]*"}); - // add("proc_uptime", {"/proc/uptime", "(.*)[\n]*"}); - - // osName may contain quotes on openSUSE - if (get("osName").find('"') == 0) - { - std::string contents = get("osName"); - size_t pos_end_quote = contents.rfind('"'); - if (pos_end_quote != std::string::npos && pos_end_quote > 0) - { - cache["osName"] = contents.substr(1, pos_end_quote - 1); - } - } + add("devId", { "/etc/machine-id", sysinfo_selector::raw}); + add("osName", {"/etc/os-release", sysinfo_selector::key_value, "ID"}); + add("osVer", {"/etc/os-release", sysinfo_selector::key_value, "VERSION_ID"}); + add("osRel", {"/etc/os-release", sysinfo_selector::key_value, "VERSION"}); + add("osBuild", {"/proc/version", sysinfo_selector::first_line}); time_t t = time(NULL); @@ -268,9 +325,9 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() #if defined(__MINGW32__) || defined(__MSYS__) // Obtain MinGW Device ID from registry - add("devId", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/ComputerHardwareId", "*"}); - add("devMake", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemManufacturer", "*"}); - add("devModel", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemProductName", "*"}); + add("devId", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/ComputerHardwareId", sysinfo_selector::raw}); + add("devMake", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemManufacturer", sysinfo_selector::raw}); + add("devModel", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemProductName", sysinfo_selector::raw}); #endif #if defined(__APPLE__) @@ -316,7 +373,7 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() } #ifndef __APPLE__ - add("appId", {"/proc/self/cmdline", "(.*)[ ]*.*[\n]*"}); + add("appId", {"/proc/self/cmdline", sysinfo_selector::first_null}); #else cache["appId"] = get_app_name(); #endif diff --git a/lib/pal/posix/sysinfo_sources.hpp b/lib/pal/posix/sysinfo_sources.hpp index 427ac31f9..041e98440 100644 --- a/lib/pal/posix/sysinfo_sources.hpp +++ b/lib/pal/posix/sysinfo_sources.hpp @@ -11,10 +11,24 @@ /** * System information source path and selector */ -typedef struct { +enum class sysinfo_selector +{ + raw, + first_line, + first_null, + key_value +}; + +struct sysinfo_source_t { const char * path; - const char * selector; -} sysinfo_source_t; + sysinfo_selector selector; + const char* name; + + sysinfo_source_t(const char* path, sysinfo_selector selector, const char* name = nullptr) + : path(path), selector(selector), name(name) + { + } +}; /** * Helper class to retrieve various key-value pairs from system info sources. @@ -29,7 +43,7 @@ class sysinfo_sources : public std::multimap { std::map cache; /** - * Read node value, preprocess it using regexp and store result in cache + * Read a bounded node value, select it without regex and store it in cache * * @param key Field name * @return true if field value is found and saved in cache @@ -60,4 +74,3 @@ class sysinfo_sources : public std::multimap { }; #endif /* LIB_PAL_POSIX_SYSINFO_SOURCES_HPP_ */ - diff --git a/lib/pal/posix/sysinfo_utils_apple.cpp b/lib/pal/posix/sysinfo_utils_apple.cpp index 7b2e71ce9..5c469f996 100644 --- a/lib/pal/posix/sysinfo_utils_apple.cpp +++ b/lib/pal/posix/sysinfo_utils_apple.cpp @@ -10,8 +10,8 @@ std::string get_sysctl_value(const char* key) { std::string result{}; // OS version will not be longer than 9 chars (XX.YY.ZZ + null), and the OS build is also 9 (nnXmmmmY + null) - char value[10]; - size_t size = sizeof(value); + char value[11] = {}; + size_t size = sizeof(value) - 1; if (sysctlbyname(key, value, &size, NULL, 0) == 0) { result.assign(value); @@ -24,4 +24,3 @@ std::string GetDeviceOsBuild() { return get_sysctl_value("kern.osversion"); } - diff --git a/lib/pal/posix/sysinfo_utils_ios.mm b/lib/pal/posix/sysinfo_utils_ios.mm index 9c0b6311b..4631793bb 100644 --- a/lib/pal/posix/sysinfo_utils_ios.mm +++ b/lib/pal/posix/sysinfo_utils_ios.mm @@ -4,6 +4,7 @@ // #include "sysinfo_utils_apple.hpp" +#include "utils/Utils.hpp" #import #include #import @@ -34,13 +35,13 @@ NSString* modelId = NSProcessInfo.processInfo.environment[@"SIMULATOR_MODEL_IDENTIFIER"]; if (modelId.length > 0) { - return std::string([modelId UTF8String]); + return MAT::boundedSystemInfo([modelId UTF8String]); } NSString* fallbackModel = [[UIDevice currentDevice] model]; if (fallbackModel.length > 0) { - return std::string([fallbackModel UTF8String]); + return MAT::boundedSystemInfo([fallbackModel UTF8String]); } return {}; @@ -50,7 +51,7 @@ if (uname(&systemInfo) < 0) { // Fallback to UIDevice in case of error - deviceModel = [[[UIDevice currentDevice] model] UTF8String]; + deviceModel = MAT::boundedSystemInfo([[[UIDevice currentDevice] model] UTF8String]); } else { @@ -92,14 +93,14 @@ { // Previous implementation pointed to "ProductVersion" on SystemVersion.plist, returning version string in format .. // systemVersion returns string in this same format - return std::string { [[[UIDevice currentDevice] systemVersion] UTF8String] }; + return MAT::boundedSystemInfo([[[UIDevice currentDevice] systemVersion] UTF8String]); } std::string GetDeviceOsRelease() { // Previous implementation pointed to "ProductUserVisibleVersion" on SystemVersion.plist, returning version string in format .. // systemVersion returns string in this same format - return std::string { [[[UIDevice currentDevice] systemVersion] UTF8String] }; + return MAT::boundedSystemInfo([[[UIDevice currentDevice] systemVersion] UTF8String]); } std::string GetDeviceClass() { diff --git a/lib/pal/posix/sysinfo_utils_mac.mm b/lib/pal/posix/sysinfo_utils_mac.mm index 193187fd3..a8a506b99 100644 --- a/lib/pal/posix/sysinfo_utils_mac.mm +++ b/lib/pal/posix/sysinfo_utils_mac.mm @@ -4,6 +4,8 @@ // #include "sysinfo_utils_apple.hpp" +#include "pal/PAL.hpp" +#include "utils/Utils.hpp" #include #include #include @@ -14,16 +16,22 @@ std::string GetDeviceModel() { + using PAL::getMATSDKLogComponent; static const char *query = "hw.model"; size_t size = 0; std::vector deviceModelBuffer; std::string deviceModel { }; if (sysctlbyname(query, nullptr, &size, nullptr, 0) == 0) { - deviceModelBuffer.resize(size); + if (size == 0 || size > MAT::MAX_SYSTEM_INFO_SOURCE_SIZE) + { + LOG_WARN("Device model exceeds system information source limit or is empty"); + return {}; + } + deviceModelBuffer.resize(size + 1, '\0'); if (sysctlbyname(query, deviceModelBuffer.data(), &size, nullptr, 0) == 0) { - deviceModel = deviceModelBuffer.data(); + deviceModel = MAT::boundedSystemInfo(deviceModelBuffer.data()); } } diff --git a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp index 486945d09..48ec00a22 100644 --- a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp @@ -55,8 +55,8 @@ namespace PAL_NS_BEGIN { m_os_architecture = WindowsEnvironmentInfo::GetProcessorArchitecture(); auto easClientDeviceInformation = ref new ::Windows::Security::ExchangeActiveSyncProvisioning::EasClientDeviceInformation(); - m_model = FromPlatformString(easClientDeviceInformation->SystemProductName); - m_manufacturer = FromPlatformString(easClientDeviceInformation->SystemManufacturer); + m_model = BoundedFromPlatformString(easClientDeviceInformation->SystemProductName); + m_manufacturer = BoundedFromPlatformString(easClientDeviceInformation->SystemManufacturer); bool isNetDetectEnabled = configuration[CFG_BOOL_ENABLE_NET_DETECT]; m_device_id = DEFAULT_DEVICE_ID; @@ -149,4 +149,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp index fa7ac6575..39075c210 100644 --- a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp @@ -38,7 +38,7 @@ namespace PAL_NS_BEGIN { { auto version = Package::Current->Id->Version; - m_app_id = FromPlatformString(Package::Current->Id->Name); + m_app_id = BoundedFromPlatformString(Package::Current->Id->Name); m_app_version = std::to_string(version.Major) + "." + std::to_string(version.Minor) + "." + std::to_string(version.Build) + "." + std::to_string(version.Revision); } else @@ -49,7 +49,7 @@ namespace PAL_NS_BEGIN { m_user_language = "en"; try { if (GlobalizationPreferences::Languages->Size) - m_user_language = FromPlatformString(GlobalizationPreferences::Languages->GetAt(0)); + m_user_language = BoundedFromPlatformString(GlobalizationPreferences::Languages->GetAt(0)); m_user_timezone = WindowsEnvironmentInfo::GetTimeZone(); } catch (AccessDeniedException^) @@ -62,7 +62,7 @@ namespace PAL_NS_BEGIN { try { - m_user_advertising_id = FromPlatformString(AdvertisingManager::AdvertisingId); + m_user_advertising_id = BoundedFromPlatformString(AdvertisingManager::AdvertisingId); } catch (Exception^) { @@ -105,7 +105,7 @@ namespace PAL_NS_BEGIN { m_os_major_version = "10.0"; } - if (FromPlatformString(AnalyticsInfo::VersionInfo->DeviceFamily) == DeviceFamily_Mobile) + if (BoundedFromPlatformString(AnalyticsInfo::VersionInfo->DeviceFamily) == DeviceFamily_Mobile) { m_os_name = WindowsPhoneOSName; } @@ -123,7 +123,7 @@ namespace PAL_NS_BEGIN { catch (...) { //CTDEBUGLOG("Language detection may not be reliable!"); } - m_app_language = FromPlatformString(primaryLanguage); + m_app_language = BoundedFromPlatformString(primaryLanguage); // CTDEBUGLOG("m_app_language=%s", m_app_language.c_str()); } diff --git a/lib/shared/PlatformHelpers.h b/lib/shared/PlatformHelpers.h index 222ee0024..ff792a9a2 100644 --- a/lib/shared/PlatformHelpers.h +++ b/lib/shared/PlatformHelpers.h @@ -225,6 +225,13 @@ namespace Microsoft { return std::string(); } +#ifdef _WINRT_DLL + inline std::string BoundedFromPlatformString(String^ platformString) + { + return MAT::boundedSystemInfo(platformString == nullptr ? nullptr : platformString->Data()); + } +#endif + inline String^ ToPlatformString(const std::string& string) { return platform_new String(UTF8ToWstring(string).c_str()); diff --git a/lib/utils/FileUtils.cpp b/lib/utils/FileUtils.cpp index 506bee9b3..ba1d04b78 100644 --- a/lib/utils/FileUtils.cpp +++ b/lib/utils/FileUtils.cpp @@ -97,28 +97,35 @@ namespace MAT_NS_BEGIN * Read file contents into std::string. * * @param UTF-8 flename - * @return File contents. Supports only UTF-8 or ASCII-encoded text files. + * @return File contents up to MAX_FILE_CONTENTS_SIZE bytes, or empty on + * read failure/oversize. Supports UTF-8 or ASCII text files. */ std::string FileGetContents(const char *filename) { -#ifdef _WIN32 - char buff[256] = { 0 }; std::string result; FILE* fp = FileOpen(filename, "r"); if (fp != nullptr) { - while (fgets(buff, sizeof(buff), fp)) + char buff[256]; + size_t count; + while ((count = fread(buff, 1, sizeof(buff), fp)) != 0) + { + if (count > MAX_FILE_CONTENTS_SIZE - result.size()) + { + LOG_WARN("File contents exceed %zu bytes; rejecting", MAX_FILE_CONTENTS_SIZE); + FileClose(fp); + return {}; + } + result.append(buff, count); + } + if (ferror(fp)) { - result += buff; + LOG_WARN("Unable to read file contents"); + result.clear(); } FileClose(fp); } return result; -#else - std::ifstream t(filename); - std::string str((std::istreambuf_iterator(t)), std::istreambuf_iterator()); - return str; -#endif } /** @@ -170,4 +177,3 @@ namespace MAT_NS_BEGIN } } MAT_NS_END - diff --git a/lib/utils/FileUtils.hpp b/lib/utils/FileUtils.hpp index cefce7627..61b22a560 100644 --- a/lib/utils/FileUtils.hpp +++ b/lib/utils/FileUtils.hpp @@ -11,6 +11,8 @@ namespace MAT_NS_BEGIN { + static constexpr size_t MAX_FILE_CONTENTS_SIZE = 4096; + size_t FileGetSize(const char* filename); int FileDelete(const char* filename); std::FILE* FileOpen(const char* filename, const char *mode); @@ -22,4 +24,3 @@ namespace MAT_NS_BEGIN } MAT_NS_END #endif - diff --git a/lib/utils/Utils.cpp b/lib/utils/Utils.cpp index a1cf48ee7..d77b0b6bc 100644 --- a/lib/utils/Utils.cpp +++ b/lib/utils/Utils.cpp @@ -40,6 +40,52 @@ namespace MAT_NS_BEGIN { namespace MAT_NS_BEGIN { + std::string boundedSystemInfo(const char* value) + { + if (value == nullptr) + { + return {}; + } + size_t length = 0; + while (length <= MAX_SYSTEM_INFO_VALUE_SIZE && value[length] != '\0') + { + ++length; + } + if (length > MAX_SYSTEM_INFO_VALUE_SIZE) + { + LOG_WARN("System information value exceeds %zu bytes; truncating", MAX_SYSTEM_INFO_VALUE_SIZE); + length = MAX_SYSTEM_INFO_VALUE_SIZE; + // Do not split a UTF-8 sequence at the truncation boundary. + while (length > 0 && (static_cast(value[length]) & 0xc0) == 0x80) + { + --length; + } + } + return std::string(value, length); + } + +#ifdef _WIN32 + std::string boundedSystemInfo(const wchar_t* value) + { + if (value == nullptr) + { + return {}; + } + size_t length = 0; + while (length <= MAX_SYSTEM_INFO_VALUE_SIZE && value[length] != L'\0') + ++length; + if (length > MAX_SYSTEM_INFO_VALUE_SIZE) + { + LOG_WARN("System information value exceeds %zu UTF-16 units; truncating", MAX_SYSTEM_INFO_VALUE_SIZE); + length = MAX_SYSTEM_INFO_VALUE_SIZE; + if (value[length - 1] >= 0xd800 && value[length - 1] <= 0xdbff) + --length; + } + const std::string utf8 = to_utf8_string(std::wstring(value, length)); + return boundedSystemInfo(utf8.c_str()); + } +#endif + void sleep(unsigned delayMs) { PAL::sleep(delayMs); diff --git a/lib/utils/Utils.hpp b/lib/utils/Utils.hpp index ce249b9ab..245ef74e1 100644 --- a/lib/utils/Utils.hpp +++ b/lib/utils/Utils.hpp @@ -55,6 +55,16 @@ namespace std namespace MAT_NS_BEGIN { + static constexpr size_t MAX_SYSTEM_INFO_VALUE_SIZE = 4096; + static constexpr size_t MAX_SYSTEM_INFO_SOURCE_SIZE = 64 * 1024; + static constexpr size_t MAX_SYSTEM_INFO_BLOB_SIZE = 1024 * 1024; + + // Bounds SDK-collected UTF-8 metadata, not caller-supplied event properties. + std::string boundedSystemInfo(const char* value); +#ifdef _WIN32 + std::string boundedSystemInfo(const wchar_t* value); +#endif + const char* getMATSDKLogComponent(); typedef std::chrono::milliseconds ms; @@ -99,4 +109,3 @@ namespace MAT_NS_BEGIN { } MAT_NS_END #endif - diff --git a/lib/utils/ZlibUtils.cpp b/lib/utils/ZlibUtils.cpp index 993aad2a8..8f567ab3f 100644 --- a/lib/utils/ZlibUtils.cpp +++ b/lib/utils/ZlibUtils.cpp @@ -5,6 +5,7 @@ #include "mat/config.h" #include "ZlibUtils.hpp" #include "pal/PAL.hpp" +#include #ifdef HAVE_MAT_ZLIB #define ZLIB_CONST @@ -13,10 +14,17 @@ namespace MAT_NS_BEGIN { + constexpr std::size_t ZlibUtils::MAX_INFLATED_SIZE; + bool ZlibUtils::InflateVector(const std::vector& in, std::vector& out, bool isGzip) { #ifdef HAVE_MAT_ZLIB bool result = true; + if (in.size() > MAX_INFLATED_SIZE || out.size() > MAX_INFLATED_SIZE) + { + LOG_WARN("Compressed payload exceeds decoder size limit"); + return false; + } z_stream zs; memset(&zs, 0, sizeof(zs)); @@ -32,19 +40,20 @@ namespace MAT_NS_BEGIN zs.next_in = (Bytef *)in.data(); zs.avail_in = (uInt)in.size(); int ret; - // The problem with 32K is that it's too small and causes corruption - // in zlib inflate. 128KB seems to be fine. - // Allocate a buffer enough to hold an output with Zlib max compression - // ratio 5:1 in case it is larger than 128KB. - uInt outbufferSize = std::max((uInt)131072, zs.avail_in * 5); - - char* outbuffer = new char[outbufferSize]; + std::array outbuffer; do { - zs.next_out = reinterpret_cast(outbuffer); - zs.avail_out = outbufferSize; + zs.next_out = outbuffer.data(); + zs.avail_out = static_cast(outbuffer.size()); ret = inflate(&zs, Z_NO_FLUSH); - out.insert(out.end(), outbuffer, outbuffer + (outbufferSize - zs.avail_out)); + const size_t count = outbuffer.size() - zs.avail_out; + if (count > MAX_INFLATED_SIZE - out.size()) + { + LOG_WARN("Inflated payload exceeds %zu bytes; rejecting", MAX_INFLATED_SIZE); + result = false; + break; + } + out.insert(out.end(), outbuffer.data(), outbuffer.data() + count); } while (ret == Z_OK); if (ret != Z_STREAM_END) { @@ -52,7 +61,6 @@ namespace MAT_NS_BEGIN result = false; } inflateEnd(&zs); - delete[] outbuffer; return result; #else UNREFERENCED_PARAMETER(in); diff --git a/lib/utils/ZlibUtils.hpp b/lib/utils/ZlibUtils.hpp index d89c4ada1..58465ff02 100644 --- a/lib/utils/ZlibUtils.hpp +++ b/lib/utils/ZlibUtils.hpp @@ -7,6 +7,7 @@ #include "ctmacros.hpp" #include +#include #include namespace MAT_NS_BEGIN @@ -14,6 +15,7 @@ namespace MAT_NS_BEGIN class ZlibUtils { public: + static constexpr std::size_t MAX_INFLATED_SIZE = 64u * 1024u * 1024u; static bool InflateVector(const std::vector& in, std::vector& out, bool isGzip); }; diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index bcdb0acb8..3b5396b89 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -6,6 +6,7 @@ set(SRCS AnnexKTests.cpp BackoffTests_ExponentialWithJitter.cpp BondSplicerTests.cpp + bounded-input-tests.cpp ClockSkewManagerTests.cpp ContextFieldsProviderTests.cpp ControlPlaneProviderTests.cpp diff --git a/tests/unittests/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index 23598d772..88c12eb16 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/tests/unittests/HttpClientCurlTests.cpp @@ -99,12 +99,34 @@ TEST(HttpClientCurlOperationTests, ClampsConnectionTimeoutBeforeMillisecondsConv std::numeric_limits::max() / 1000L); } +class CurlResponseBufferCallbacks : public CurlHttpOperation +{ +public: + using CurlHttpOperation::WriteHeaderCallback; +}; + +TEST(HttpClientCurlOperationTests, EnforcesExactHeaderBufferLimitWithoutPartialWrites) +{ + std::vector input(MAX_HTTP_RESPONSE_HEADERS_SIZE, 'x'); + std::vector output; + EXPECT_EQ(CurlResponseBufferCallbacks::WriteHeaderCallback( + input.data(), 1, input.size(), &output), input.size()); + EXPECT_EQ(output.size(), MAX_HTTP_RESPONSE_HEADERS_SIZE); + EXPECT_EQ(CurlResponseBufferCallbacks::WriteHeaderCallback( + input.data(), 1, 1, &output), 0u); + EXPECT_EQ(output.size(), MAX_HTTP_RESPONSE_HEADERS_SIZE); + EXPECT_EQ(CurlResponseBufferCallbacks::WriteHeaderCallback( + input.data(), 2, std::numeric_limits::max(), &output), 0u); + EXPECT_EQ(output.size(), MAX_HTTP_RESPONSE_HEADERS_SIZE); +} + class HttpClientCurlHeaderTests : public ::testing::Test, public HttpServer::Callback { protected: HttpServer m_server; std::string m_url; + std::map m_responseHeaders {{"X-MAT-Test", "header-value"}}; void SetUp() override { @@ -124,7 +146,7 @@ class HttpClientCurlHeaderTests : public ::testing::Test, int onHttpRequest(HttpServer::Request const&, HttpServer::Response& response) override { - response.headers["X-MAT-Test"] = "header-value"; + response.headers = m_responseHeaders; response.content = "body-value"; return 200; } @@ -149,6 +171,34 @@ TEST_F(HttpClientCurlHeaderTests, CapturesResponseHeadersAndBody) EXPECT_EQ(std::string(responseBody.begin(), responseBody.end()), "body-value"); } +TEST_F(HttpClientCurlHeaderTests, ParsesLongHeaderValuesLinearlyAndPreservesColons) +{ + const std::string value = std::string(16 * 1024, 'x') + ": keep: value"; + m_responseHeaders["X-Long"] = value; + CurlHttpOperation operation("GET", m_url, nullptr, {}, {}); + operation.Send(); + ASSERT_EQ(operation.GetTransportError(), CURLE_OK); + EXPECT_EQ(operation.GetResponseHeaders().at("X-Long"), value); +} + +TEST_F(HttpClientCurlHeaderTests, RejectsOversizedAggregateHeaders) +{ + for (int i = 0; i < 5; ++i) + m_responseHeaders["X-Long-" + std::to_string(i)] = std::string(16 * 1024, 'x'); + CurlHttpOperation operation("GET", m_url, nullptr, {}, {}); + operation.Send(); + EXPECT_EQ(operation.GetTransportError(), CURLE_WRITE_ERROR); +} + +TEST_F(HttpClientCurlHeaderTests, RejectsOversizedHeadersInRawResponseMode) +{ + for (int i = 0; i < 5; ++i) + m_responseHeaders["X-Long-" + std::to_string(i)] = std::string(16 * 1024, 'x'); + CurlHttpOperation operation("GET", m_url, nullptr, {}, {}, true); + operation.Send(); + EXPECT_EQ(operation.GetTransportError(), CURLE_WRITE_ERROR); +} + // --- ILogConfiguration integration --- TEST(HttpClientCurlConfigTests, LogConfiguration_SslVerify_DefaultIsTrue) diff --git a/tests/unittests/HttpClientTests.cpp b/tests/unittests/HttpClientTests.cpp index d73280d5d..06cb861b3 100644 --- a/tests/unittests/HttpClientTests.cpp +++ b/tests/unittests/HttpClientTests.cpp @@ -108,6 +108,7 @@ class HttpClientTests : public ::testing::Test, _server.addHandler("/count/", *this); _server.addHandler("/block/", *this); _server.addHandler("/large/", *this); + _server.addHandler("/large-headers/", *this); _server.addHandler("/redirect/", *this); _server.addHandler("/cookie/", *this); _server.addHandler("/query", *this); @@ -195,6 +196,13 @@ class HttpClientTests : public ::testing::Test, return 200; } + if (request.uri == "/large-headers/") { + for (int i = 0; i < 5; ++i) { + inResponse.headers["X-Long-" + std::to_string(i)] = std::string(16 * 1024, 'x'); + } + return 200; + } + if (request.uri.substr(0, 7) == "/count/") { int id = atoi(request.uri.substr(7).c_str()); if (id >= 0 && static_cast(id) < _countedRequests.size()) { @@ -1114,6 +1122,19 @@ TEST_F(HttpClientTests, HandlesResponseLargerThanReadBuffer) EXPECT_THAT(response->GetBody(), Eq(Binary(LargePayload(responseSize)))); } +TEST_F(HttpClientTests, RejectsOversizedResponseHeaders) +{ + std::unique_ptr request(_client->CreateRequest()); + request->SetUrl("http://" + _hostname + "/large-headers/"); + _client->SendRequestAsync(request.release(), this); + std::unique_lock lock(_lock); + ASSERT_TRUE(_responseCv.wait_for(lock, std::chrono::seconds(30), + [this]() { return !_responses.empty(); })); + ASSERT_EQ(_responses.size(), 1u); + EXPECT_EQ(_responses[0]->GetResult(), HttpResult_NetworkFailure); + EXPECT_TRUE(_responses[0]->GetBody().empty()); +} + TEST_F(HttpClientTests, HandlesRequestAndResponseLargerThanReadBuffer) { Clear(); diff --git a/tests/unittests/LogSessionDataTests.cpp b/tests/unittests/LogSessionDataTests.cpp index 47a7f035a..7dc30d6f6 100644 --- a/tests/unittests/LogSessionDataTests.cpp +++ b/tests/unittests/LogSessionDataTests.cpp @@ -6,6 +6,7 @@ #include "common/Common.hpp" #include "offline/LogSessionDataProvider.hpp" #include +#include "utils/FileUtils.hpp" using namespace testing; using namespace Microsoft::Applications::Events; @@ -67,6 +68,13 @@ TEST(LogSessionDataTests, parse_ValidInput_ReturnsTrue) ASSERT_EQ(sessionSDKUid, "bar"); } +TEST(LogSessionDataTests, parse_OversizedInput_ReturnsFalse) +{ + TestLogSessionDataProvider provider(PathToTestSesFile); + const std::string content = "1234567890\n" + std::string(MAX_FILE_CONTENTS_SIZE, 'x') + "\n"; + ASSERT_FALSE(provider.parse(content, sessionFirstTimeLaunch, sessionSDKUid)); +} + TEST(LogSessionDataTests, getLogSessionData_ValidInput_SessionDataPersists) { const std::string sessionFile = diff --git a/tests/unittests/PayloadDecoderTests.cpp b/tests/unittests/PayloadDecoderTests.cpp index 8e8051cee..0975fbc82 100644 --- a/tests/unittests/PayloadDecoderTests.cpp +++ b/tests/unittests/PayloadDecoderTests.cpp @@ -4,10 +4,27 @@ // #include "common/Common.hpp" #include "PayloadDecoder.hpp" +#include "utils/ZlibUtils.hpp" using namespace testing; using namespace MAT; +TEST(PayloadDecoderTests, RejectsOversizedRequestsBeforeCopying) +{ + std::vector input(ZlibUtils::MAX_INFLATED_SIZE + 1, 0); + std::string output = "previous"; + EXPECT_FALSE(exporters::DecodeRequest(input, output, false)); + EXPECT_TRUE(output.empty()); +} + +TEST(PayloadDecoderTests, RejectsShortMalformedInputWithoutReadingPastTheEnd) +{ + const std::vector input {0xff}; + std::string output; + EXPECT_FALSE(exporters::DecodeRequest(input, output, false)); + EXPECT_TRUE(output.empty()); +} + namespace { // Builds a minimally-populated Common Schema record. to_json() in the diff --git a/tests/unittests/ZlibUtilsTests.cpp b/tests/unittests/ZlibUtilsTests.cpp index 54822e775..dd1a2df61 100644 --- a/tests/unittests/ZlibUtilsTests.cpp +++ b/tests/unittests/ZlibUtilsTests.cpp @@ -6,6 +6,44 @@ using namespace testing; using namespace MAT; + +TEST(ZlibUtilsTests, RejectsDecompressionBeyondTheOutputLimit) +{ + // A small compressed input must not be able to grow an already-full output. + std::vector input = {0x03, 0x00}; + std::vector output(ZlibUtils::MAX_INFLATED_SIZE + 1, 0); + EXPECT_FALSE(ZlibUtils::InflateVector(input, output, false)); +} + +TEST(ZlibUtilsTests, BoundsHighlyCompressiblePayloads) +{ + std::vector original(ZlibUtils::MAX_INFLATED_SIZE + 1, 'x'); + std::vector compressed(compressBound(static_cast(original.size()))); + uLongf size = static_cast(compressed.size()); + ASSERT_EQ(compress2(compressed.data(), &size, original.data(), + static_cast(original.size()), Z_BEST_SPEED), Z_OK); + // compress2 writes a zlib header and checksum, not raw deflate. + compressed.erase(compressed.begin(), compressed.begin() + 2); + compressed.resize(static_cast(size) - 6); + std::vector output; + EXPECT_FALSE(ZlibUtils::InflateVector(compressed, output, false)); + EXPECT_LE(output.size(), ZlibUtils::MAX_INFLATED_SIZE); +} + +TEST(ZlibUtilsTests, AcceptsAnExactlyFullInflatedPayload) +{ + std::vector original(ZlibUtils::MAX_INFLATED_SIZE, 'x'); + std::vector compressed(compressBound(static_cast(original.size()))); + uLongf size = static_cast(compressed.size()); + ASSERT_EQ(compress2(compressed.data(), &size, original.data(), + static_cast(original.size()), Z_BEST_SPEED), Z_OK); + compressed.erase(compressed.begin(), compressed.begin() + 2); + compressed.resize(static_cast(size) - 6); + std::vector output; + EXPECT_TRUE(ZlibUtils::InflateVector(compressed, output, false)); + EXPECT_EQ(output, original); +} + TEST(ZlibUtilsTests, InflateVector) { std::vector uncompressed(682440, 99); diff --git a/tests/unittests/bounded-input-tests.cpp b/tests/unittests/bounded-input-tests.cpp new file mode 100644 index 000000000..82ac268ee --- /dev/null +++ b/tests/unittests/bounded-input-tests.cpp @@ -0,0 +1,207 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// + +#include "bond/All.hpp" +#include "bond/CompactBinaryProtocolReader.hpp" +#include "bond/generated/BondConstTypes.hpp" +#include "common/Common.hpp" +#include "utils/FileUtils.hpp" +#include "utils/Utils.hpp" + +#if !defined(_WIN32) && !defined(ANDROID) +#include "pal/posix/sysinfo_sources.hpp" +#endif +#if defined(__linux__) && !defined(ANDROID) +#include +#include +#include +#endif + +using namespace MAT; + +class BoundedInputTests : public testing::Test +{ + protected: + std::string path = GetTempDirectory() + "bounded-input-" + PAL::generateUuidString(); + + ~BoundedInputTests() override + { + FileDelete(path.c_str()); + } + + void write(const std::string& contents) + { + FILE* file = FileOpen(path.c_str(), "wb"); + ASSERT_NE(file, nullptr); + const size_t written = fwrite(contents.data(), 1, contents.size(), file); + const int closed = FileClose(file); + ASSERT_EQ(written, contents.size()); + ASSERT_EQ(closed, 0); + } +}; + +TEST_F(BoundedInputTests, BoundsMetadataAtExactByteLimit) +{ + EXPECT_EQ(boundedSystemInfo(static_cast(nullptr)), ""); + EXPECT_EQ(boundedSystemInfo("ordinary"), "ordinary"); + const std::string exact(MAX_SYSTEM_INFO_VALUE_SIZE, 'x'); + EXPECT_EQ(boundedSystemInfo(exact.c_str()), exact); + EXPECT_EQ(boundedSystemInfo((exact + "extra").c_str()), exact); +} + +TEST_F(BoundedInputTests, DoesNotSplitUtf8Metadata) +{ + const std::string prefix(MAX_SYSTEM_INFO_VALUE_SIZE - 1, 'x'); + EXPECT_EQ(boundedSystemInfo((prefix + "\xc3\xa9").c_str()), prefix); + const std::string exact(MAX_SYSTEM_INFO_VALUE_SIZE - 2, 'x'); + EXPECT_EQ(boundedSystemInfo((exact + "\xc3\xa9" + "z").c_str()), exact + "\xc3\xa9"); +} + +#ifdef _WIN32 +TEST_F(BoundedInputTests, BoundsUtf16BeforeConversion) +{ + const std::wstring input(MAX_SYSTEM_INFO_VALUE_SIZE + 1, L'x'); + EXPECT_EQ(boundedSystemInfo(input.c_str()), std::string(MAX_SYSTEM_INFO_VALUE_SIZE, 'x')); + const std::wstring unicode(MAX_SYSTEM_INFO_VALUE_SIZE, L'\u00e9'); + EXPECT_EQ(boundedSystemInfo(unicode.c_str()).size(), MAX_SYSTEM_INFO_VALUE_SIZE); +} +#endif + +TEST_F(BoundedInputTests, RejectsOversizedFilesInsteadOfParsingAPrefix) +{ + const std::string exact(MAX_FILE_CONTENTS_SIZE, 'x'); + write(exact); + EXPECT_EQ(FileGetContents(path.c_str()), exact); + write(exact + "x"); + EXPECT_TRUE(FileGetContents(path.c_str()).empty()); + write(std::string(100 * 1024, 'x')); + EXPECT_TRUE(FileGetContents(path.c_str()).empty()); +} + +TEST_F(BoundedInputTests, RejectsImpossibleContainerCountsBeforeAllocation) +{ + const std::vector input = {bond_lite::BT_STRING, 0xff, 0xff, 0xff, 0xff, 0x0f}; + bond_lite::CompactBinaryProtocolReader reader(input); + uint32_t size; + uint8_t type; + EXPECT_FALSE(reader.ReadContainerBegin(size, type)); +} + +TEST_F(BoundedInputTests, EnforcesContainerElementLimit) +{ + std::vector input = {bond_lite::BT_BOOL, 0x80, 0x80, 0x04}; + input.resize(input.size() + bond_lite::CompactBinaryProtocolReader::MAX_CONTAINER_ELEMENTS, 0); + bond_lite::CompactBinaryProtocolReader reader(input); + uint32_t size; + uint8_t type; + EXPECT_TRUE(reader.ReadContainerBegin(size, type)); + input[1] = 0x81; + input.push_back(0); + bond_lite::CompactBinaryProtocolReader oversizedReader(input); + EXPECT_FALSE(oversizedReader.ReadContainerBegin(size, type)); +} + +#if !defined(_WIN32) && !defined(ANDROID) +TEST_F(BoundedInputTests, ReadsOnlyArgvZeroFromLongNulSeparatedCommandLine) +{ + const std::string executable = "/opt/app with spaces\nin-name"; + write(executable + '\0' + std::string(100 * 1024, 'x')); + sysinfo_sources sources; + sources.add("appId", {path.c_str(), sysinfo_selector::first_null}); + EXPECT_EQ(sources.get("appId"), executable); +} + +TEST_F(BoundedInputTests, BoundsLongArgvZeroAndRawMetadata) +{ + write(std::string(100 * 1024, 'x') + '\0' + "argument"); + sysinfo_sources sources; + sources.add("appId", {path.c_str(), sysinfo_selector::first_null}); + sources.add("raw", {path.c_str(), sysinfo_selector::raw}); + EXPECT_EQ(sources.get("appId"), std::string(MAX_SYSTEM_INFO_VALUE_SIZE, 'x')); + EXPECT_EQ(sources.get("raw").size(), MAX_SYSTEM_INFO_VALUE_SIZE); +} + +TEST_F(BoundedInputTests, ReadsOsReleaseKeysWithoutSuffixMatchesOrRegex) +{ + write( + "VERSION_ID=\"24.04\"\nID_LIKE=wrong\n# ID=wrong\nID=\"ubuntu\"\n" + "VERSION='24.04 (LTS)'\n"); + sysinfo_sources sources; + sources.add("osName", {path.c_str(), sysinfo_selector::key_value, "ID"}); + sources.add("osVer", {path.c_str(), sysinfo_selector::key_value, "VERSION_ID"}); + sources.add("osRel", {path.c_str(), sysinfo_selector::key_value, "VERSION"}); + EXPECT_EQ(sources.get("osName"), "ubuntu"); + EXPECT_EQ(sources.get("osVer"), "24.04"); + EXPECT_EQ(sources.get("osRel"), "24.04 (LTS)"); +} + +TEST_F(BoundedInputTests, BoundsOsReleaseValuesAndReadsUnterminatedFinalLine) +{ + write("ID=linux\nVERSION=" + std::string(100 * 1024, 'x')); + sysinfo_sources sources; + sources.add("version", {path.c_str(), sysinfo_selector::key_value, "VERSION"}); + EXPECT_EQ(sources.get("version").size(), MAX_SYSTEM_INFO_VALUE_SIZE); + write("ID=linux"); + sysinfo_sources finalLine; + finalLine.add("id", {path.c_str(), sysinfo_selector::key_value, "ID"}); + EXPECT_EQ(finalLine.get("id"), "linux"); +} + +TEST_F(BoundedInputTests, ReadsOnlyFirstLineAndRetainsCache) +{ + write("Linux build\n" + std::string(100 * 1024, 'x')); + sysinfo_sources sources; + sources.add("build", {path.c_str(), sysinfo_selector::first_line}); + EXPECT_EQ(sources.get("build"), "Linux build"); + write("changed"); + EXPECT_EQ(sources.get("build"), "Linux build"); +} +#endif + +#if defined(__linux__) && !defined(ANDROID) +TEST_F(BoundedInputTests, ActualCommandLineOnSmallStack) +{ + sysinfo_sources sources; + sources.add("appId", {"/proc/self/cmdline", sysinfo_selector::first_null}); + std::string result; + struct State + { + sysinfo_sources& sources; + std::string& result; + } state{sources, result}; + pthread_attr_t attributes; + ASSERT_EQ(pthread_attr_init(&attributes), 0); + ASSERT_EQ(pthread_attr_setstacksize(&attributes, 64 * 1024), 0); + pthread_t thread; + const int created = pthread_create(&thread, &attributes, [](void* pointer) -> void* + { + auto& state = *static_cast(pointer); + state.result = state.sources.get("appId"); + return nullptr; }, &state); + pthread_attr_destroy(&attributes); + ASSERT_EQ(created, 0); + ASSERT_EQ(pthread_join(thread, nullptr), 0); + EXPECT_FALSE(result.empty()); + EXPECT_EQ(result.find('\0'), std::string::npos); + EXPECT_LE(result.size(), MAX_SYSTEM_INFO_VALUE_SIZE); +} + +TEST_F(BoundedInputTests, InitializesTelemetryWithA100KbCommandLine) +{ + std::string padding(100 * 1024, 'x'); + const char* filter = "--gtest_filter=BoundedInputTests.ActualCommandLineOnSmallStack"; + const pid_t child = fork(); + ASSERT_NE(child, -1); + if (child == 0) + { + execl("/proc/self/exe", "/proc/self/exe", filter, padding.c_str(), static_cast(nullptr)); + _exit(127); + } + int status; + ASSERT_EQ(waitpid(child, &status, 0), child); + ASSERT_TRUE(WIFEXITED(status)); + EXPECT_EQ(WEXITSTATUS(status), 0); +} +#endif From d0947dce81306116361ed2b23cbdb58471093618 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 11:34:43 -0500 Subject: [PATCH 2/9] Allow downstream apps to omit native device-ID collection Compile out native device-ID collection so downstream applications can provide their own IDs without redundant platform reads. Preserve default behavior, expose the setting through the vcpkg overlay, and cover disabled collectors and Android error paths. Files changed: - .github/workflows/build-posix-latest.yml - CMakeLists.txt - cmake/MatsdkOptions.cmake - docs/PAL.md - docs/building-with-vcpkg.md - docs/embedding-with-cmake.md - lib/CMakeLists.txt - lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java - lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/EventsUnitTest.java - lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java - lib/pal/PAL.cpp - lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp - lib/pal/posix/DeviceInformationImpl.cpp - lib/pal/posix/DeviceInformationImpl_Android.cpp - lib/pal/posix/sysinfo_sources.cpp - lib/pal/posix/sysinfo_utils_ios.mm - lib/pal/posix/sysinfo_utils_mac.mm - lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp - tests/unittests/CMakeLists.txt - tests/unittests/ContextFieldsProviderTests.cpp - tests/unittests/device-id-tests.cpp - tests/vcpkg/device-id-feature-tests.cmake - tools/ports/cpp-client-telemetry/portfile.cmake - tools/ports/cpp-client-telemetry/vcpkg.json Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- .github/workflows/build-posix-latest.yml | 43 +++++++++++ CMakeLists.txt | 4 + cmake/MatsdkOptions.cmake | 2 + docs/PAL.md | 19 +++++ docs/building-with-vcpkg.md | 40 ++++++++++ docs/embedding-with-cmake.md | 6 ++ lib/CMakeLists.txt | 2 +- .../applications/events/HttpClient.java | 9 ++- .../applications/events/EventsUnitTest.java | 45 ++++++++++- .../events/HttpClientRequestTest.java | 5 ++ lib/pal/PAL.cpp | 6 +- .../WindowsDesktopDeviceInformationImpl.cpp | 10 ++- lib/pal/posix/DeviceInformationImpl.cpp | 3 +- .../posix/DeviceInformationImpl_Android.cpp | 35 ++++++--- lib/pal/posix/sysinfo_sources.cpp | 20 ++--- lib/pal/posix/sysinfo_utils_ios.mm | 4 + lib/pal/posix/sysinfo_utils_mac.mm | 6 ++ .../WindowsRuntimeDeviceInformationImpl.cpp | 2 + tests/unittests/CMakeLists.txt | 1 + .../unittests/ContextFieldsProviderTests.cpp | 4 + tests/unittests/device-id-tests.cpp | 76 +++++++++++++++++++ tests/vcpkg/device-id-feature-tests.cmake | 73 ++++++++++++++++++ .../ports/cpp-client-telemetry/portfile.cmake | 14 ++++ tools/ports/cpp-client-telemetry/vcpkg.json | 4 + 24 files changed, 403 insertions(+), 30 deletions(-) create mode 100644 tests/unittests/device-id-tests.cpp create mode 100644 tests/vcpkg/device-id-feature-tests.cmake diff --git a/.github/workflows/build-posix-latest.yml b/.github/workflows/build-posix-latest.yml index 95c387fd0..5451ea0db 100644 --- a/.github/workflows/build-posix-latest.yml +++ b/.github/workflows/build-posix-latest.yml @@ -49,6 +49,49 @@ jobs: - name: Test ${{ matrix.os }} ${{ matrix.config }} run: ./build-tests.sh ${{ matrix.config }} + device-id-optout: + name: Device-ID opt-out (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macOS-latest, windows-2022] + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - name: Initialize googletest + run: git submodule update --init --depth=1 third_party/googletest + - name: Configure without native device-ID collection + shell: pwsh + run: | + $options = @( + "-S", ".", "-B", "out-device-id-off", + "-DCMAKE_BUILD_TYPE=Debug", + "-DMATSDK_ENABLE_DEVICE_ID=OFF", + "-DMATSDK_BUILD_UNIT_TESTS=ON", + "-DMATSDK_BUILD_FUNC_TESTS=OFF", + "-DMATSDK_BUILD_TEST_TOOL=OFF", + "-DMATSDK_BUILD_PACKAGE=OFF" + ) + if ($env:RUNNER_OS -eq "Windows") { + $options += "-DCMAKE_CXX_FLAGS_INIT=/EHsc" + } + cmake @options + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + - name: Build + run: cmake --build out-device-id-off --config Debug --target UnitTests --parallel 2 + - name: Run affected and adjacent suites + shell: pwsh + run: | + $binary = "out-device-id-off/tests/unittests/UnitTests" + if ($env:RUNNER_OS -eq "Windows") { + $binary = "out-device-id-off\tests\unittests\Debug\UnitTests.exe" + } + & $binary "--gtest_filter=DeviceIdTests.*:ContextFieldsProviderTests.*:BoundedInputTests.*:PalTests.*:SysInfoUtilsTests.*" + exit $LASTEXITCODE + - name: Verify vcpkg feature mapping + run: cmake -P tests/vcpkg/device-id-feature-tests.cmake + public-headers: name: Public header gate (GCC/Clang) runs-on: ubuntu-latest diff --git a/CMakeLists.txt b/CMakeLists.txt index 9dadc7a70..7835483cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -120,6 +120,10 @@ if(MATSDK_DISABLE_LOGGING) target_compile_definitions(matsdk_internal_config INTERFACE MATSDK_DISABLE_LOGGING) endif() +if(NOT MATSDK_ENABLE_DEVICE_ID) + target_compile_definitions(matsdk_internal_config INTERFACE + MATSDK_DISABLE_DEVICE_ID) +endif() option(GCC5_CXX11_ABI_WORKAROUND "Workaround: Use legacy C++11 ABI (for GCC 5 compatibility)" OFF) if(GCC5_CXX11_ABI_WORKAROUND) diff --git a/cmake/MatsdkOptions.cmake b/cmake/MatsdkOptions.cmake index ed95ec805..71a400ab1 100644 --- a/cmake/MatsdkOptions.cmake +++ b/cmake/MatsdkOptions.cmake @@ -46,6 +46,8 @@ option(MATSDK_USE_WININET "Use WinInet instead of WinHTTP as the Win32 desktop HTTP client" OFF) option(MATSDK_DISABLE_LOGGING "Compile internal SDK logging out" OFF) +option(MATSDK_ENABLE_DEVICE_ID + "Collect a device ID using the SDK's platform-specific implementation" ON) set(_matsdk_android_http_client_predefined OFF) if(DEFINED MATSDK_ANDROID_HTTP_CLIENT) diff --git a/docs/PAL.md b/docs/PAL.md index 8dbb438f7..0b4bbf1f7 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -59,6 +59,25 @@ ## Remote configuration (ECS client) +## Device-ID collection + +`MATSDK_ENABLE_DEVICE_ID` defaults to `ON`. Configure with +`-DMATSDK_ENABLE_DEVICE_ID=OFF` to compile out the SDK's native device-ID +collectors, including machine-ID reads, fallback shell commands, adapter +queries, host UUIDs, and vendor/Android identifiers. Other system and device +metadata remain enabled. The macOS build also stops linking the ID-specific +IOKit framework; dependencies still used by other features remain. + +Without a supplied ID, `DeviceInfo.Id` is omitted rather than replaced with a +placeholder. Applications can still supply their own ID through +`ISemanticContext::SetDeviceId`; registration does not overwrite it with an +empty automatically collected ID. Android's Java bridge consults the native +build setting before accessing `ANDROID_ID`; use the matching Java bridge +sources with the native SDK. + +This option does not disable session/SDK identifiers or control device IDs +added independently by the operating system's UTC telemetry pipeline. + ## Input size limits Automatically collected system, device, application, and network-provider diff --git a/docs/building-with-vcpkg.md b/docs/building-with-vcpkg.md index a4aa85a3c..41e9ad2e7 100644 --- a/docs/building-with-vcpkg.md +++ b/docs/building-with-vcpkg.md @@ -57,6 +57,46 @@ project's `vcpkg.json`: ## Platform-Specific Instructions +### Disabling SDK device-ID collection + +The in-repo overlay port has a default-enabled `device-id` feature, mapped to +the default-on CMake option `MATSDK_ENABLE_DEVICE_ID`. To supply your own +`DeviceInfo.Id` without running the SDK's native collectors, omit that feature +and explicitly retain the other features you need: + +```json +{ + "dependencies": [ + { + "name": "cpp-client-telemetry", + "default-features": false, + "features": ["curl-openssl", "system-sqlite"] + } + ] +} +``` + +For classic mode: + +```console +vcpkg install "cpp-client-telemetry[core,curl-openssl,system-sqlite]" --overlay-ports=tools/ports +``` + +`curl-openssl` affects Linux only; Android still defaults to Java transport. +`[core,...]` and `default-features: false` drop **all** defaults, including +`device-id`. Add `device-id` explicitly when combining non-default features +but wanting to retain native ID collection. + +vcpkg features are additive across the dependency graph: another dependency +requesting `device-id` (or the default features) re-enables collection. Ensure +all requests for this port agree when opting out. + +The official registry requires a port update pointing to an SDK release +containing this option; existing registry versions do not expose the feature. +Until then, use this repository's overlay port and source checkout. The port +rejects an opt-out against an older SDK rather than silently collecting an ID. +For direct CMake builds, use `-DMATSDK_ENABLE_DEVICE_ID=OFF`. + ### Windows ```powershell diff --git a/docs/embedding-with-cmake.md b/docs/embedding-with-cmake.md index 6cce81468..692d91579 100644 --- a/docs/embedding-with-cmake.md +++ b/docs/embedding-with-cmake.md @@ -29,6 +29,12 @@ Set it explicitly to `ON` in consumer CI to test new toolchains strictly. Set `MATSDK_DISABLE_LOGGING=ON` to compile the SDK's internal diagnostic logging out. This does not disable telemetry event collection or upload. +Set `MATSDK_ENABLE_DEVICE_ID=OFF` before adding the SDK to compile out native +device-ID collection while retaining other metadata. It defaults to `ON`. +Applications can still set their own ID through `ISemanticContext::SetDeviceId`; +without one, `DeviceInfo.Id` is omitted. See [device-ID collection](PAL.md#device-id-collection) +for platform details and the distinction from session/SDK identifiers. + ## SQLite and zlib providers Source builds can select dependency modes without patching 1DS sources: diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index d311be424..b77acdf41 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -701,7 +701,7 @@ elseif(APPLE) ) if(MATSDK_PLATFORM_IOS) target_link_libraries(mat PUBLIC "-framework UIKit") - else() + elseif(MATSDK_ENABLE_DEVICE_ID) target_link_libraries(mat PUBLIC "-framework IOKit") endif() endif() diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java index 221edce2f..b310b74a5 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java @@ -227,7 +227,7 @@ public HttpClient(Context context, @NonNull HttpClientRequest.Factory requestFac String path = System.getProperty("java.io.tmpdir"); setCacheFilePath(path); if (configuration.isInitializeDeviceInfo()) { - setDeviceInfo(calculateID(context), Build.MANUFACTURER, Build.MODEL); + setDeviceInfo(calculateID(context, isDeviceIdCollectionEnabled()), Build.MANUFACTURER, Build.MODEL); } calculateAndSetSystemInfo(context); m_executor = createExecutor(); @@ -317,7 +317,10 @@ private void calculateAndSetSystemInfo(android.content.Context context) { device_class); } - private String calculateID(android.content.Context context) { + static String calculateID(android.content.Context context, boolean collectDeviceId) { + if (!collectDeviceId) { + return ""; + } // The definition of ANDROID_ID changed in API 26. // https://developer.android.com/reference/android/provider/Settings.Secure#ANDROID_ID @@ -379,6 +382,8 @@ public URL newUrl(String url) throws java.net.MalformedURLException { public native void setDeviceInfo(String id, String manufacturer, String model); + public native boolean isDeviceIdCollectionEnabled(); + public native void setSystemInfo( String app_id, String app_version, diff --git a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/EventsUnitTest.java b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/EventsUnitTest.java index 513354682..9f9e0c498 100644 --- a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/EventsUnitTest.java +++ b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/EventsUnitTest.java @@ -15,10 +15,12 @@ import android.net.ConnectivityManager; import android.net.NetworkCapabilities; import android.os.BatteryManager; +import android.util.Log; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; @@ -49,6 +51,7 @@ import static org.mockito.Mockito.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.isA; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -151,6 +154,7 @@ public void dispatchCallback(String id, int response, Object[] headers, byte[] b else { byte[] emptiness = {}; assertArrayEquals(emptiness, body); + assertEquals(0, headers.length); } } @@ -165,6 +169,11 @@ protected ExecutorService createExecutor() return mockExecutor; } + @Override + public boolean isDeviceIdCollectionEnabled() { + return true; + } + @Override public void setDeviceInfo(String id, String manufacturer, String model) { @@ -363,7 +372,8 @@ public void requestException() throws java.io.IOException, PackageManager.NameNo when(mockUrl.openConnection()).thenReturn(mockConnection); when(mockConnection.getOutputStream()).thenReturn(mockBodyStream); - when(mockConnection.getResponseCode()).thenThrow(new IOException("Space Aliens")); + IOException failure = new IOException("Space Aliens"); + when(mockConnection.getResponseCode()).thenThrow(failure); connectMocks(); int previousDispatch = dispatchCount.get(); Stubby stubs = new Stubby(mockContext); @@ -378,8 +388,11 @@ public void requestException() throws java.io.IOException, PackageManager.NameNo ); assertNotNull(task); assertEquals(previousDispatch, dispatchCount.get()); - expectedResponse = 0; - task.run(); + expectedResponse = -1; + try (MockedStatic log = mockStatic(Log.class)) { + task.run(); + log.verify(() -> Log.e("MAE", "HTTP response failed", failure)); + } assertEquals(previousDispatch + 1, dispatchCount.get()); assertFalse(task.isCancelled()); assertTrue(task.isDone()); @@ -390,6 +403,32 @@ public void requestException() throws java.io.IOException, PackageManager.NameNo } } + @Test + public void responseReadExceptionDoesNotRetainSuccessfulStatus() throws java.io.IOException, PackageManager.NameNotFoundException { + when(mockUrl.openConnection()).thenReturn(mockConnection); + when(mockConnection.getOutputStream()).thenReturn(mockBodyStream); + when(mockConnection.getResponseCode()).thenReturn(200); + Map> headerMap = new TreeMap<>(); + headerMap.put("foo", java.util.Collections.singletonList("bar")); + when(mockConnection.getHeaderFields()).thenReturn(headerMap); + IOException failure = new IOException("Response read failed"); + when(mockConnection.getInputStream()).thenThrow(failure); + connectMocks(); + int previousDispatch = dispatchCount.get(); + Stubby stubs = new Stubby(mockContext); + FutureTask task = stubs.createTask( + "https://www.contoso.com", "POST", new byte[] {0, 1, 2}, + "failed-response", new int[] {4, 3}, new byte[] {0, 1, 2, 3, 7, 8, 9} + ); + expectedResponse = -1; + try (MockedStatic log = mockStatic(Log.class)) { + task.run(); + log.verify(() -> Log.e("MAE", "HTTP response failed", failure)); + } + assertEquals(previousDispatch + 1, dispatchCount.get()); + assertTrue(task.isDone()); + } + @Test public void newEventPropertiesStorageEventTypeDefaultsToEmptyString() { // Regression test for #1329: EventPropertiesStorage previously left diff --git a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java index 4bca802f2..7430ccc06 100644 --- a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java +++ b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java @@ -21,6 +21,11 @@ import static org.junit.Assert.*; public class HttpClientRequestTest { + @Test + public void disabledDeviceIdDoesNotAccessAndroidContext() { + Assert.assertEquals("", HttpClient.calculateID(null, false)); + } + @Test public void acceptsExactlyFullResponseBody() throws IOException { byte[] body = new byte[Request.MAX_HTTP_RESPONSE_SIZE]; diff --git a/lib/pal/PAL.cpp b/lib/pal/PAL.cpp index ad471b0ce..3fc1fad0f 100644 --- a/lib/pal/PAL.cpp +++ b/lib/pal/PAL.cpp @@ -568,7 +568,11 @@ namespace PAL_NS_BEGIN { { if (m_DeviceInformation != nullptr) { - context->SetDeviceId(MAT::boundedSystemInfo(m_DeviceInformation->GetDeviceId().c_str())); + const auto& deviceId = m_DeviceInformation->GetDeviceId(); +#ifdef MATSDK_DISABLE_DEVICE_ID + if (!deviceId.empty()) +#endif + context->SetDeviceId(MAT::boundedSystemInfo(deviceId.c_str())); context->SetDeviceModel(MAT::boundedSystemInfo(m_DeviceInformation->GetModel().c_str())); context->SetDeviceMake(MAT::boundedSystemInfo(m_DeviceInformation->GetManufacturer().c_str())); } diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index 0cc145092..d0b539497 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -14,13 +14,15 @@ MATSDK_LOG_INST_COMPONENT_NS("DeviceInfo", "Win32 Desktop Device Information") +#ifndef MATSDK_DISABLE_DEVICE_ID #include +#pragma comment(lib, "iphlpapi.lib") +#endif #include #include #include #include -#pragma comment(lib, "iphlpapi.lib") #pragma comment(lib, "AdvAPI32.Lib") #define MALLOC(x) HeapAlloc(GetProcessHeap(), 0, (x)) @@ -35,11 +37,14 @@ using namespace MAT; namespace PAL_NS_BEGIN { +#ifndef MATSDK_DISABLE_DEVICE_ID /* Value returned on computers with no network adapter available */ static const char *devIdDefault = "{deadbeef-fade-dead-c0de-cafebabefeed}"; +#endif static const char *manufacturer = "Unknown Manufacturer"; static const char *model = "Unknown Model"; +#ifndef MATSDK_DISABLE_DEVICE_ID /** * Returns the GUID of the 1st network adapter. */ @@ -82,6 +87,7 @@ namespace PAL_NS_BEGIN { } return devId; } +#endif // Helper functions. PowerSource GetCurrentPowerSource() @@ -116,7 +122,9 @@ namespace PAL_NS_BEGIN { { m_os_architecture = WindowsEnvironmentInfo::GetProcessorArchitecture(); +#ifndef MATSDK_DISABLE_DEVICE_ID m_device_id = getDeviceId(); +#endif char buff[256] = { 0 }; DWORD size = sizeof(buff); diff --git a/lib/pal/posix/DeviceInformationImpl.cpp b/lib/pal/posix/DeviceInformationImpl.cpp index 606536d93..a39b9d9f7 100644 --- a/lib/pal/posix/DeviceInformationImpl.cpp +++ b/lib/pal/posix/DeviceInformationImpl.cpp @@ -40,8 +40,10 @@ namespace PAL_NS_BEGIN { #endif auto sysInfo = sysinfo_sources_impl::GetSysInfo(); +#ifndef MATSDK_DISABLE_DEVICE_ID std::string devId = sysInfo.get("devId"); m_device_id = (devId.empty()) ? DEFAULT_DEVICE_ID : devId; +#endif m_manufacturer = sysInfo.get("devMake"); @@ -71,4 +73,3 @@ namespace PAL_NS_BEGIN { DeviceInformationImpl::~DeviceInformationImpl() {} } PAL_NS_END - diff --git a/lib/pal/posix/DeviceInformationImpl_Android.cpp b/lib/pal/posix/DeviceInformationImpl_Android.cpp index 991c6fa3e..1a249c92c 100644 --- a/lib/pal/posix/DeviceInformationImpl_Android.cpp +++ b/lib/pal/posix/DeviceInformationImpl_Android.cpp @@ -45,7 +45,11 @@ namespace PAL_NS_BEGIN { }; PowerSource AndroidDeviceInformationConnector::s_power_source = PowerSource_Unknown; +#ifndef MATSDK_DISABLE_DEVICE_ID std::string AndroidDeviceInformationConnector::s_device_id = DEFAULT_DEVICE_ID; +#else + std::string AndroidDeviceInformationConnector::s_device_id; +#endif std::string AndroidDeviceInformationConnector::s_manufacturer; std::string AndroidDeviceInformationConnector::s_model; std::mutex AndroidDeviceInformationConnector::s_registered_mutex; @@ -171,6 +175,15 @@ namespace PAL_NS_BEGIN { } jclass buildClass = pEnv->FindClass("android/os/Build"); + jfieldID manufacturerFid = pEnv->GetStaticFieldID(buildClass, "MANUFACTURER", "Ljava/lang/String;"); + jfieldID modelFid = pEnv->GetStaticFieldID(buildClass, "MODEL", "Ljava/lang/String;"); + jstring manufacturerJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildClass, manufacturerFid)); + jstring modelJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildClass, modelFid)); + + const char* jStr; + jboolean isCopy; + +#ifndef MATSDK_DISABLE_DEVICE_ID jclass contextClass = pEnv->FindClass("android/content/Context"); jclass secureSettingsClass = pEnv->FindClass("android/provider/Settings$Secure"); @@ -182,26 +195,20 @@ namespace PAL_NS_BEGIN { jmethodID getContentResolverMid = pEnv->GetMethodID(contextClass, "getContentResolver", "()Landroid/content/ContentResolver;"); - jfieldID manufacturerFid = pEnv->GetStaticFieldID(buildClass, "MANUFACTURER", "Ljava/lang/String;"); - jfieldID modelFid = pEnv->GetStaticFieldID(buildClass, "MODEL", "Ljava/lang/String;"); jfieldID androidIdSettingFid = pEnv->GetStaticFieldID(secureSettingsClass, "ANDROID_ID", "Ljava/lang/String;"); - - jstring manufacturerJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildClass, manufacturerFid)); - jstring modelJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildClass, modelFid)); jstring androidIdSettingJstr = reinterpret_cast(pEnv->GetStaticObjectField(secureSettingsClass, androidIdSettingFid)); jobject contentResolver = pEnv->CallObjectMethod(activity, getContentResolverMid); jstring androidIdJstr = reinterpret_cast( pEnv->CallStaticObjectMethod(secureSettingsClass, getStringMid, contentResolver, androidIdSettingJstr)); - const char* jStr; - jboolean isCopy; - jStr = pEnv->GetStringUTFChars(androidIdJstr, &isCopy); std::string deviceId("a:"); deviceId += MAT::boundedSystemInfo(jStr); deviceId = MAT::boundedSystemInfo(deviceId.c_str()); pEnv->ReleaseStringUTFChars(androidIdJstr, jStr); + AndroidDeviceInformationConnector::setDeviceId(std::move(deviceId)); +#endif jStr = pEnv->GetStringUTFChars(manufacturerJstr, &isCopy); std::string manufacturer = MAT::boundedSystemInfo(jStr); @@ -211,12 +218,22 @@ namespace PAL_NS_BEGIN { std::string model = MAT::boundedSystemInfo(jStr); pEnv->ReleaseStringUTFChars(modelJstr, jStr); - AndroidDeviceInformationConnector::setDeviceId(std::move(deviceId)); AndroidDeviceInformationConnector::setManufacturer(std::move(manufacturer)); AndroidDeviceInformationConnector::setModel(std::move(model)); } } PAL_NS_END +extern "C" JNIEXPORT jboolean JNICALL +Java_com_microsoft_applications_events_HttpClient_isDeviceIdCollectionEnabled( + JNIEnv*, jobject) +{ +#ifdef MATSDK_DISABLE_DEVICE_ID + return JNI_FALSE; +#else + return JNI_TRUE; +#endif +} + extern "C" JNIEXPORT void diff --git a/lib/pal/posix/sysinfo_sources.cpp b/lib/pal/posix/sysinfo_sources.cpp index a3c2d9c8b..64a5b9ce5 100644 --- a/lib/pal/posix/sysinfo_sources.cpp +++ b/lib/pal/posix/sysinfo_sources.cpp @@ -55,7 +55,7 @@ #include "TargetConditionals.h" #include "sysinfo_utils_apple.hpp" -#ifdef TARGET_MAC_OS +#if defined(TARGET_MAC_OS) && !defined(MATSDK_DISABLE_DEVICE_ID) #include @@ -142,13 +142,7 @@ inline std::string ReadFile(const char *filename, sysinfo_selector selector) * @param cmd Command to execute * @return output */ -#if defined(__clang__) -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wunused-function" // Used on non-Apple platforms. See sysinfo_sources_impl() -#elif defined(__GNUC__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wunused-function" // Used on non-Apple platforms. See sysinfo_sources_impl() -#endif +#if !defined(__APPLE__) && !defined(MATSDK_DISABLE_DEVICE_ID) static std::string Exec(const char* cmd) { using PAL::getMATSDKLogComponent; @@ -190,10 +184,6 @@ static std::string Exec(const char* cmd) return result; } -#if defined(__clang__) -#pragma clang diagnostic pop -#elif defined(__GNUC__) -#pragma GCC diagnostic pop #endif /** @@ -288,7 +278,9 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() uname(&buf); #if defined(__linux__) // Obtain Linux system information from filesystem +#ifndef MATSDK_DISABLE_DEVICE_ID add("devId", { "/etc/machine-id", sysinfo_selector::raw}); +#endif add("osName", {"/etc/os-release", sysinfo_selector::key_value, "ID"}); add("osVer", {"/etc/os-release", sysinfo_selector::key_value, "VERSION_ID"}); add("osRel", {"/etc/os-release", sysinfo_selector::key_value, "VERSION"}); @@ -325,7 +317,9 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() #if defined(__MINGW32__) || defined(__MSYS__) // Obtain MinGW Device ID from registry +#ifndef MATSDK_DISABLE_DEVICE_ID add("devId", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/ComputerHardwareId", sysinfo_selector::raw}); +#endif add("devMake", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemManufacturer", sysinfo_selector::raw}); add("devModel", { "/proc/registry/HKEY_LOCAL_MACHINE/SYSTEM/CurrentControlSet/Control/SystemInformation/SystemProductName", sysinfo_selector::raw}); #endif @@ -378,6 +372,7 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() cache["appId"] = get_app_name(); #endif +#ifndef MATSDK_DISABLE_DEVICE_ID if (!get("devId").compare("")) { #ifdef __APPLE__ @@ -405,5 +400,6 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() } #endif } +#endif } diff --git a/lib/pal/posix/sysinfo_utils_ios.mm b/lib/pal/posix/sysinfo_utils_ios.mm index 4631793bb..5bc446e13 100644 --- a/lib/pal/posix/sysinfo_utils_ios.mm +++ b/lib/pal/posix/sysinfo_utils_ios.mm @@ -74,6 +74,7 @@ std::string GetDeviceId() { +#ifndef MATSDK_DISABLE_DEVICE_ID @autoreleasepool { NSUUID *nsuuid = [[UIDevice currentDevice] identifierForVendor]; if (nsuuid) @@ -87,6 +88,9 @@ return emptyString; } } +#else + return {}; +#endif } std::string GetDeviceOsVersion() diff --git a/lib/pal/posix/sysinfo_utils_mac.mm b/lib/pal/posix/sysinfo_utils_mac.mm index a8a506b99..0134dbccf 100644 --- a/lib/pal/posix/sysinfo_utils_mac.mm +++ b/lib/pal/posix/sysinfo_utils_mac.mm @@ -8,7 +8,9 @@ #include "utils/Utils.hpp" #include #include +#ifndef MATSDK_DISABLE_DEVICE_ID #include +#endif #include #import @@ -45,6 +47,7 @@ std::string GetDeviceId() { +#ifndef MATSDK_DISABLE_DEVICE_ID uuid_t uuidBytes; const struct timespec spec = {1, 0}; int hostUUIDResult = gethostuuid(uuidBytes, &spec); @@ -57,6 +60,9 @@ } return {EMPTY_GUID}; +#else + return {}; +#endif } std::string GetDeviceOsVersion() diff --git a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp index 48ec00a22..e683a6d3a 100644 --- a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp @@ -58,6 +58,7 @@ namespace PAL_NS_BEGIN { m_model = BoundedFromPlatformString(easClientDeviceInformation->SystemProductName); m_manufacturer = BoundedFromPlatformString(easClientDeviceInformation->SystemManufacturer); +#ifndef MATSDK_DISABLE_DEVICE_ID bool isNetDetectEnabled = configuration[CFG_BOOL_ENABLE_NET_DETECT]; m_device_id = DEFAULT_DEVICE_ID; @@ -86,6 +87,7 @@ namespace PAL_NS_BEGIN { // Workaround for Windows OS bug VSO: 11314171 - sometimes NetworkInformation triggers exception m_device_id = DEFAULT_DEVICE_ID; } +#endif #ifdef _WIN32_WINNT_WIN10 #else // Windows 8.1 SDK diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 3b5396b89..14502a66f 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -13,6 +13,7 @@ set(SRCS CorrelationVectorTests.cpp DebugEventSourceTests.cpp DeviceStateHandlerTests.cpp + device-id-tests.cpp DiskLocalStorageTests.cpp EventFilterCollectionTests.cpp EventPropertiesDecoratorTests.cpp diff --git a/tests/unittests/ContextFieldsProviderTests.cpp b/tests/unittests/ContextFieldsProviderTests.cpp index a9c91e7cb..a75c32e72 100644 --- a/tests/unittests/ContextFieldsProviderTests.cpp +++ b/tests/unittests/ContextFieldsProviderTests.cpp @@ -126,7 +126,11 @@ TEST(ContextFieldsProviderTests, UsesPalValues) ::CsProtocol::Record record; ctx.writeToRecord(record); +#ifdef MATSDK_DISABLE_DEVICE_ID + EXPECT_THAT(record.extDevice[0].localId, IsEmpty()); +#else EXPECT_THAT(record.extDevice[0].localId, Not(IsEmpty())); +#endif //EXPECT_THAT(record.extDevice[0].authSecId, Not(IsEmpty())); //EXPECT_THAT(record.data[0].properties["DeviceInfo.NetworkType"].stringValue, Not(IsEmpty())); EXPECT_THAT(record.extOs[0].name, Not(IsEmpty())); diff --git a/tests/unittests/device-id-tests.cpp b/tests/unittests/device-id-tests.cpp new file mode 100644 index 000000000..ad368125c --- /dev/null +++ b/tests/unittests/device-id-tests.cpp @@ -0,0 +1,76 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// + +#include "common/Common.hpp" + +#include "api/ContextFieldsProvider.hpp" +#include "utils/Utils.hpp" + +#if !defined(_WIN32) && !defined(ANDROID) +#include "pal/posix/sysinfo_sources_impl.hpp" +#endif +#ifdef __APPLE__ +#include "pal/posix/sysinfo_utils_apple.hpp" +#endif + +using namespace MAT; + +TEST(DeviceIdTests, NativeCollectorFollowsBuildOption) +{ + auto device = PAL::GetDeviceInformation(); + ASSERT_NE(device, nullptr); +#ifdef MATSDK_DISABLE_DEVICE_ID + EXPECT_TRUE(device->GetDeviceId().empty()); +#if !defined(_WIN32) && !defined(ANDROID) + EXPECT_TRUE(sysinfo_sources_impl::GetSysInfo().get("devId").empty()); +#endif +#ifdef __APPLE__ + EXPECT_TRUE(GetDeviceId().empty()); +#endif +#else + EXPECT_FALSE(device->GetDeviceId().empty()); +#endif +} + +TEST(DeviceIdTests, SemanticRegistrationPreservesCallerIdWhenCollectionIsDisabled) +{ + ContextFieldsProvider context; + const std::string callerId = "c:" + std::string(8 * 1024, 'x'); + context.SetDeviceId(callerId); + PAL::registerSemanticContext(&context); + + const auto& fields = context.GetCommonFields(); + ASSERT_EQ(fields.count(COMMONFIELDS_DEVICE_ID), 1u); +#ifdef MATSDK_DISABLE_DEVICE_ID + const std::string expected = callerId; +#else + const std::string expected = boundedSystemInfo(PAL::GetDeviceInformation()->GetDeviceId().c_str()); +#endif + EXPECT_EQ(std::string(fields.at(COMMONFIELDS_DEVICE_ID).as_string), expected); +#ifdef MATSDK_DISABLE_DEVICE_ID + CsProtocol::Record record; + context.writeToRecord(record); + ASSERT_FALSE(record.extDevice.empty()); + EXPECT_EQ(record.extDevice[0].localId, expected); +#endif +} + +TEST(DeviceIdTests, RegistrationRetainsOtherMetadataWithoutInventingAnId) +{ + ContextFieldsProvider context; + PAL::registerSemanticContext(&context); + const auto& fields = context.GetCommonFields(); +#ifdef MATSDK_DISABLE_DEVICE_ID + EXPECT_EQ(fields.count(COMMONFIELDS_DEVICE_ID), 0u); +#else + EXPECT_EQ(fields.count(COMMONFIELDS_DEVICE_ID), 1u); +#endif + ASSERT_EQ(fields.count(COMMONFIELDS_OS_NAME), 1u); + ASSERT_EQ(fields.count(COMMONFIELDS_APP_ID), 1u); + EXPECT_EQ(std::string(fields.at(COMMONFIELDS_OS_NAME).as_string), + boundedSystemInfo(PAL::GetSystemInformation()->GetOsName().c_str())); + EXPECT_EQ(std::string(fields.at(COMMONFIELDS_APP_ID).as_string), + boundedSystemInfo(PAL::GetSystemInformation()->GetAppId().c_str())); +} diff --git a/tests/vcpkg/device-id-feature-tests.cmake b/tests/vcpkg/device-id-feature-tests.cmake new file mode 100644 index 000000000..b4b1e41aa --- /dev/null +++ b/tests/vcpkg/device-id-feature-tests.cmake @@ -0,0 +1,73 @@ +cmake_minimum_required(VERSION 3.19) + +get_filename_component(REPO_ROOT "${CMAKE_CURRENT_LIST_DIR}/../.." ABSOLUTE) +set(ENV{MATSDK_VCPKG_SOURCE_DIR} "${REPO_ROOT}") + +# Exercise the real port's feature mapping without downloading its dependencies. +function(vcpkg_cmake_configure) + cmake_parse_arguments(CONFIG "" "SOURCE_PATH" "OPTIONS" ${ARGN}) + if(NOT "-DMATSDK_ENABLE_DEVICE_ID=${EXPECTED_DEVICE_ID}" IN_LIST CONFIG_OPTIONS) + message(FATAL_ERROR "Incorrect device-ID CMake option: ${CONFIG_OPTIONS}") + endif() +endfunction() + +function(vcpkg_cmake_install) +endfunction() +function(vcpkg_cmake_config_fixup) +endfunction() +function(vcpkg_install_copyright) +endfunction() + +function(check_feature_mapping platform expected) + set(FEATURES ${ARGN}) + set(EXPECTED_DEVICE_ID "${expected}") + set(CURRENT_PORT_DIR "${REPO_ROOT}/tools/ports/cpp-client-telemetry") + set(CURRENT_PACKAGES_DIR "${REPO_ROOT}/out/device-id-feature-tests") + set(VCPKG_LIBRARY_LINKAGE static) + foreach(target LINUX WINDOWS OSX IOS ANDROID) + set(VCPKG_TARGET_IS_${target} OFF) + endforeach() + set(VCPKG_TARGET_IS_${platform} ON) + include("${CURRENT_PORT_DIR}/portfile.cmake") +endfunction() + +if(DEFINED OLD_SOURCE) + set(ENV{MATSDK_VCPKG_SOURCE_DIR} "${OLD_SOURCE}") + check_feature_mapping(LINUX OFF curl-openssl system-sqlite) + return() +endif() + +file(READ "${REPO_ROOT}/tools/ports/cpp-client-telemetry/vcpkg.json" MANIFEST) +string(JSON DEFAULT_COUNT LENGTH "${MANIFEST}" default-features) +math(EXPR DEFAULT_LAST "${DEFAULT_COUNT} - 1") +set(DEFAULT_FEATURES) +foreach(index RANGE ${DEFAULT_LAST}) + string(JSON FEATURE GET "${MANIFEST}" default-features ${index}) + list(APPEND DEFAULT_FEATURES "${FEATURE}") +endforeach() +if(NOT "device-id" IN_LIST DEFAULT_FEATURES) + message(FATAL_ERROR "Native device-ID collection must remain a default feature") +endif() +string(JSON DESCRIPTION GET "${MANIFEST}" features device-id description) +set(OPT_OUT_FEATURES ${DEFAULT_FEATURES}) +list(REMOVE_ITEM OPT_OUT_FEATURES device-id) + +foreach(platform LINUX WINDOWS OSX IOS ANDROID) + check_feature_mapping("${platform}" ON ${DEFAULT_FEATURES}) + check_feature_mapping("${platform}" OFF ${OPT_OUT_FEATURES}) +endforeach() +check_feature_mapping(LINUX OFF curl-mbedtls minimal-sqlite) + +set(OLD_SOURCE "${REPO_ROOT}/out/device-id-feature-tests/older-source") +file(MAKE_DIRECTORY "${OLD_SOURCE}") +file(WRITE "${OLD_SOURCE}/CMakeLists.txt" "cmake_minimum_required(VERSION 3.19)\n") +execute_process( + COMMAND "${CMAKE_COMMAND}" "-DOLD_SOURCE=${OLD_SOURCE}" -P "${CMAKE_CURRENT_LIST_FILE}" + RESULT_VARIABLE OLD_RESULT + OUTPUT_QUIET + ERROR_VARIABLE OLD_ERROR) +file(REMOVE "${OLD_SOURCE}/CMakeLists.txt") +if(OLD_RESULT EQUAL 0 OR NOT OLD_ERROR MATCHES "Disabling device-ID collection requires") + message(FATAL_ERROR "The port must reject an opt-out against an unsupported SDK: ${OLD_ERROR}") +endif() +message(STATUS "Device-ID vcpkg feature mapping passed") diff --git a/tools/ports/cpp-client-telemetry/portfile.cmake b/tools/ports/cpp-client-telemetry/portfile.cmake index a4c229d52..b6ff32379 100644 --- a/tools/ports/cpp-client-telemetry/portfile.cmake +++ b/tools/ports/cpp-client-telemetry/portfile.cmake @@ -139,6 +139,20 @@ if(VCPKG_TARGET_IS_WINDOWS "MATSDK_VCPKG_SOURCE_DIR to a local checkout containing that option.") endif() set(MATSDK_PINNED_SOURCE_OPTIONS) +if(MATSDK_OPTION_SOURCE MATCHES "MATSDK_ENABLE_DEVICE_ID") + set(MATSDK_ENABLE_DEVICE_ID OFF) + if("device-id" IN_LIST FEATURES) + set(MATSDK_ENABLE_DEVICE_ID ON) + endif() + list(APPEND MATSDK_PINNED_SOURCE_OPTIONS + -DMATSDK_ENABLE_DEVICE_ID=${MATSDK_ENABLE_DEVICE_ID}) +elseif(NOT "device-id" IN_LIST FEATURES) + message(FATAL_ERROR + "Disabling device-ID collection requires a cpp-client-telemetry source " + "revision that supports MATSDK_ENABLE_DEVICE_ID. Update this port's " + "REF/SHA512 to a newer SDK release, or set MATSDK_VCPKG_SOURCE_DIR " + "to a local checkout containing that option.") +endif() if(MATSDK_ROOT_CMAKE MATCHES "MATSDK_USE_VCPKG_DEPS") list(APPEND MATSDK_PINNED_SOURCE_OPTIONS -DMATSDK_USE_VCPKG_DEPS=ON) endif() diff --git a/tools/ports/cpp-client-telemetry/vcpkg.json b/tools/ports/cpp-client-telemetry/vcpkg.json index 14ab091c5..c467f869f 100644 --- a/tools/ports/cpp-client-telemetry/vcpkg.json +++ b/tools/ports/cpp-client-telemetry/vcpkg.json @@ -22,6 +22,7 @@ ], "default-features": [ "curl-openssl", + "device-id", "system-sqlite" ], "features": { @@ -79,6 +80,9 @@ } ] }, + "device-id": { + "description": "Collect a device ID using the SDK's platform-specific implementation (default). Omit this feature with default-features=false to supply your own DeviceInfo.Id without native device-ID collection." + }, "minimal-sqlite": { "description": "Build a private, feature-stripped SQLite compiled from the SDK's vendored amalgamation instead of linking the external sqlite3 package. Smaller footprint; combine with [core,minimal-sqlite] to also drop the sqlite3 dependency." }, From 23cf112f92282779cb2c531dfd1ce144ff2f7de9 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 12:29:42 -0500 Subject: [PATCH 3/9] Provide prerequisites for device-ID opt-out CI Install Linux system development libraries before configuring the opt-out build, and enable MSVC big-object support for shared test targets so template-heavy mocks compile with debug information. Files changed: - .github/workflows/build-posix-latest.yml - tests/CMakeLists.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- .github/workflows/build-posix-latest.yml | 3 +++ tests/CMakeLists.txt | 3 +++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/build-posix-latest.yml b/.github/workflows/build-posix-latest.yml index 5451ea0db..3bdc4b955 100644 --- a/.github/workflows/build-posix-latest.yml +++ b/.github/workflows/build-posix-latest.yml @@ -61,6 +61,9 @@ jobs: uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Initialize googletest run: git submodule update --init --depth=1 third_party/googletest + - name: Install Linux development dependencies + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev libsqlite3-dev zlib1g-dev - name: Configure without native device-ID collection shell: pwsh run: | diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 216590ebd..11a4e7417 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,5 +1,8 @@ add_library(matsdk_test_config INTERFACE) target_link_libraries(matsdk_test_config INTERFACE matsdk_internal_config) +if(MSVC) + target_compile_options(matsdk_test_config INTERFACE /bigobj) +endif() target_include_directories(matsdk_test_config INTERFACE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../lib From 43a5f3d5be116d644836a721b9f40117c3b4e4c9 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 13:19:17 -0500 Subject: [PATCH 4/9] Reuse established CI paths for device-ID opt-out coverage Exercise disabled collection with the existing POSIX and Windows build/test scripts instead of maintaining separate prerequisite installation and compiler settings. Pass the Windows disable definition through the existing custom-properties hook and include the regression source in its native project. Files changed: - .github/workflows/build-posix-latest.yml - tests/CMakeLists.txt - tests/device-id-optout.props - tests/unittests/UnitTests.vcxproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- .github/workflows/build-posix-latest.yml | 47 ++++++++---------------- tests/CMakeLists.txt | 3 -- tests/device-id-optout.props | 7 ++++ tests/unittests/UnitTests.vcxproj | 1 + 4 files changed, 24 insertions(+), 34 deletions(-) create mode 100644 tests/device-id-optout.props diff --git a/.github/workflows/build-posix-latest.yml b/.github/workflows/build-posix-latest.yml index 3bdc4b955..fa75b6733 100644 --- a/.github/workflows/build-posix-latest.yml +++ b/.github/workflows/build-posix-latest.yml @@ -52,6 +52,8 @@ jobs: device-id-optout: name: Device-ID opt-out (${{ matrix.os }}) runs-on: ${{ matrix.os }} + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" strategy: fail-fast: false matrix: @@ -61,37 +63,20 @@ jobs: uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - name: Initialize googletest run: git submodule update --init --depth=1 third_party/googletest - - name: Install Linux development dependencies - if: runner.os == 'Linux' - run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev libsqlite3-dev zlib1g-dev - - name: Configure without native device-ID collection - shell: pwsh - run: | - $options = @( - "-S", ".", "-B", "out-device-id-off", - "-DCMAKE_BUILD_TYPE=Debug", - "-DMATSDK_ENABLE_DEVICE_ID=OFF", - "-DMATSDK_BUILD_UNIT_TESTS=ON", - "-DMATSDK_BUILD_FUNC_TESTS=OFF", - "-DMATSDK_BUILD_TEST_TOOL=OFF", - "-DMATSDK_BUILD_PACKAGE=OFF" - ) - if ($env:RUNNER_OS -eq "Windows") { - $options += "-DCMAKE_CXX_FLAGS_INIT=/EHsc" - } - cmake @options - if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - - name: Build - run: cmake --build out-device-id-off --config Debug --target UnitTests --parallel 2 - - name: Run affected and adjacent suites - shell: pwsh - run: | - $binary = "out-device-id-off/tests/unittests/UnitTests" - if ($env:RUNNER_OS -eq "Windows") { - $binary = "out-device-id-off\tests\unittests\Debug\UnitTests.exe" - } - & $binary "--gtest_filter=DeviceIdTests.*:ContextFieldsProviderTests.*:BoundedInputTests.*:PalTests.*:SysInfoUtilsTests.*" - exit $LASTEXITCODE + - name: Test Linux/macOS without native device-ID collection + if: runner.os != 'Windows' + env: + CMAKE_OPTS: -DMATSDK_ENABLE_DEVICE_ID=OFF + run: ./build-tests.sh debug + - name: setup-msbuild + if: runner.os == 'Windows' + uses: microsoft/setup-msbuild@6fb02220983dee41ce7ae257b6f4d8f9bf5ed4ce # v2.0.0 + with: + vs-version: '[17,)' + - name: Test Windows without native device-ID collection + if: runner.os == 'Windows' + shell: cmd + run: build-tests.cmd x64 Debug "tests\device-id-optout.props" WinHTTP - name: Verify vcpkg feature mapping run: cmake -P tests/vcpkg/device-id-feature-tests.cmake diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 11a4e7417..216590ebd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,8 +1,5 @@ add_library(matsdk_test_config INTERFACE) target_link_libraries(matsdk_test_config INTERFACE matsdk_internal_config) -if(MSVC) - target_compile_options(matsdk_test_config INTERFACE /bigobj) -endif() target_include_directories(matsdk_test_config INTERFACE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/../lib diff --git a/tests/device-id-optout.props b/tests/device-id-optout.props new file mode 100644 index 000000000..43827de31 --- /dev/null +++ b/tests/device-id-optout.props @@ -0,0 +1,7 @@ + + + + MATSDK_DISABLE_DEVICE_ID;%(PreprocessorDefinitions) + + + diff --git a/tests/unittests/UnitTests.vcxproj b/tests/unittests/UnitTests.vcxproj index 89da8f8be..953718f00 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -438,6 +438,7 @@ + From 1628897b19ee2be29f0eb3088509189f63222e05 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 14:00:44 -0500 Subject: [PATCH 5/9] Address Copilot review by bounding decoder work and physical file reads Copilot comment 4186871826: eliminate repeated copies of the remaining diagnostic payload by scanning request bytes in place and copying only each record. Verified in lib/decoder/PayloadDecoder.cpp:81-109. Copilot comment 4186871913: count physical session-file bytes using binary reads, preserve CRLF parsing, and reject trailing content. Verify exact/oversize CRLF and Ctrl+Z cases plus session regeneration, including registration in the native Windows test project. Verified in lib/utils/FileUtils.cpp:102 and lib/offline/LogSessionDataProvider.cpp:166-177. Files changed: - docs/CsProtocol-decoding.md - docs/PAL.md - lib/decoder/PayloadDecoder.cpp - lib/offline/LogSessionDataProvider.cpp - lib/utils/FileUtils.cpp - tests/unittests/LogSessionDataTests.cpp - tests/unittests/PayloadDecoderTests.cpp - tests/unittests/UnitTests.vcxproj - tests/unittests/bounded-input-tests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- docs/CsProtocol-decoding.md | 2 + docs/PAL.md | 4 +- lib/decoder/PayloadDecoder.cpp | 5 +- lib/offline/LogSessionDataProvider.cpp | 6 ++- lib/utils/FileUtils.cpp | 2 +- tests/unittests/LogSessionDataTests.cpp | 61 +++++++++++++++++++++++++ tests/unittests/PayloadDecoderTests.cpp | 33 +++++++++++++ tests/unittests/UnitTests.vcxproj | 1 + tests/unittests/bounded-input-tests.cpp | 24 ++++++++++ 9 files changed, 132 insertions(+), 6 deletions(-) diff --git a/docs/CsProtocol-decoding.md b/docs/CsProtocol-decoding.md index c06d9b2ac..9a8fd1ca6 100644 --- a/docs/CsProtocol-decoding.md +++ b/docs/CsProtocol-decoding.md @@ -40,6 +40,8 @@ Binary container counts are limited to 65,536 elements and must fit the remaining encoded input (at least one byte per sequence element or two bytes per map entry). Invalid counts are rejected before generated readers allocate their containers. Requests are also limited to 65,536 decoded records. +Record-boundary scanning reads the request in place and copies only each +individual record, so scanning and input copying are linear in request size. Decoding failure returns `false` rather than reporting a partial request as success; partial inflation output must not be used after a failed call. diff --git a/docs/PAL.md b/docs/PAL.md index 0b4bbf1f7..9d3b9a5f8 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -91,12 +91,14 @@ event properties or semantic context. | POSIX OS release file and device-ID command output | 64 KiB | Bound the collected prefix and log a warning | | OS-sized executable path and device-model buffers | 64 KiB | Reject before allocating | | Windows version-resource and adapter-information buffers | 1 MiB | Reject before allocating; retain the existing missing-information fallback | -| Session sidecar file read through `FileGetContents` | 4 KiB | Reject the whole file, log a warning, and regenerate session data | +| Session sidecar file read through `FileGetContents` | 4 KiB of physical file bytes | Reject the whole file, log a warning, and regenerate session data | | HTTP response body | 16 MiB | Fail the request rather than retain an oversized response | | HTTP response headers | 64 KiB | Fail the request rather than retain oversized headers | POSIX OS release values use exact line-key matching rather than recursive regular expressions. Curl response headers are also parsed without regex. +Session files are read in binary mode so CRLF and Ctrl+Z cannot bypass the +byte limit; session parsing accepts both LF and CRLF line endings. The header budget includes framing for native raw headers or a minimum four-byte allowance per name/value pair. Windows native queries measure raw UTF-16/ANSI buffer bytes; WinRT conservatively budgets up to three UTF-8 bytes diff --git a/lib/decoder/PayloadDecoder.cpp b/lib/decoder/PayloadDecoder.cpp index b974d03c8..22c788c5d 100644 --- a/lib/decoder/PayloadDecoder.cpp +++ b/lib/decoder/PayloadDecoder.cpp @@ -78,12 +78,11 @@ namespace clienttelemetry { } Record result; length = request.size() - i; - std::vector test(request.data() + i, request.data() + i + length); size_t j = 3; bool found = false; while (j < length) { - while (j < length && test[j] != '\x3') + while (j < length && request[i + j] != '\x3') { j++; } @@ -91,7 +90,7 @@ namespace clienttelemetry { { if (j + 2 < length) { - if (test[j + 1] == ('0'+::CsProtocol::CS_VER_MAJOR) && test[j + 2] == '.') + if (request[i + j + 1] == ('0'+::CsProtocol::CS_VER_MAJOR) && request[i + j + 2] == '.') { found = true; break; diff --git a/lib/offline/LogSessionDataProvider.cpp b/lib/offline/LogSessionDataProvider.cpp index d15ab3caf..a18fd4455 100644 --- a/lib/offline/LogSessionDataProvider.cpp +++ b/lib/offline/LogSessionDataProvider.cpp @@ -152,7 +152,7 @@ namespace MAT_NS_BEGIN } std::vector v; StringUtils::SplitString(content, '\n', v); - if (v.size() != 3) { + if (v.size() != 3 || !v[2].empty()) { return false; } remove_eol(v[0]); @@ -218,6 +218,10 @@ namespace MAT_NS_BEGIN { result.erase(result.length() - 1); } + if (!result.empty() && result[result.length() - 1] == '\r') + { + result.erase(result.length() - 1); + } } } MAT_NS_END diff --git a/lib/utils/FileUtils.cpp b/lib/utils/FileUtils.cpp index ba1d04b78..60fb78de1 100644 --- a/lib/utils/FileUtils.cpp +++ b/lib/utils/FileUtils.cpp @@ -103,7 +103,7 @@ namespace MAT_NS_BEGIN std::string FileGetContents(const char *filename) { std::string result; - FILE* fp = FileOpen(filename, "r"); + FILE* fp = FileOpen(filename, "rb"); if (fp != nullptr) { char buff[256]; diff --git a/tests/unittests/LogSessionDataTests.cpp b/tests/unittests/LogSessionDataTests.cpp index 7dc30d6f6..d48c4d40b 100644 --- a/tests/unittests/LogSessionDataTests.cpp +++ b/tests/unittests/LogSessionDataTests.cpp @@ -68,6 +68,22 @@ TEST(LogSessionDataTests, parse_ValidInput_ReturnsTrue) ASSERT_EQ(sessionSDKUid, "bar"); } +TEST(LogSessionDataTests, parse_ValidCrlfInput_ReturnsTrue) +{ + TestLogSessionDataProvider provider(PathToTestSesFile); + ASSERT_TRUE(provider.parse("1234567890\r\nbar\r\n", sessionFirstTimeLaunch, sessionSDKUid)); + EXPECT_EQ(sessionFirstTimeLaunch, uint64_t{1234567890}); + EXPECT_EQ(sessionSDKUid, "bar"); +} + +TEST(LogSessionDataTests, parse_TrailingData_ReturnsFalse) +{ + TestLogSessionDataProvider provider(PathToTestSesFile); + EXPECT_FALSE(provider.parse("1234567890\nbar\nextra", sessionFirstTimeLaunch, sessionSDKUid)); + EXPECT_FALSE(provider.parse("1234567890\r\nbar\r\n\x1a" "hidden", + sessionFirstTimeLaunch, sessionSDKUid)); +} + TEST(LogSessionDataTests, parse_OversizedInput_ReturnsFalse) { TestLogSessionDataProvider provider(PathToTestSesFile); @@ -96,3 +112,48 @@ TEST(LogSessionDataTests, getLogSessionData_ValidInput_SessionDataPersists) logSessionDataProvider1.DeleteLogSessionData(); logSessionDataProvider2.DeleteLogSessionData(); } + +class LogSessionFileTests : public Test +{ +protected: + std::string cachePath = GetTempDirectory() + "session-file-" + PAL::generateUuidString(); + std::string sessionPath = cachePath + ".ses"; + + ~LogSessionFileTests() override + { + FileDelete(sessionPath.c_str()); + } + + void write(const std::string& content) + { + FILE* file = FileOpen(sessionPath.c_str(), "wb"); + ASSERT_NE(file, nullptr); + const size_t written = fwrite(content.data(), 1, content.size(), file); + const int closed = FileClose(file); + ASSERT_EQ(written, content.size()); + ASSERT_EQ(closed, 0); + } +}; + +TEST_F(LogSessionFileTests, PreservesExistingCrlfSession) +{ + write("1234567890\r\noriginal-id\r\n"); + TestLogSessionDataProvider provider(cachePath); + provider.CreateLogSessionData(); + ASSERT_NE(provider.GetLogSessionData(), nullptr); + EXPECT_EQ(provider.GetLogSessionData()->getSessionFirstTime(), uint64_t{1234567890}); + EXPECT_EQ(provider.GetLogSessionData()->getSessionSDKUid(), "original-id"); +} + +TEST_F(LogSessionFileTests, RegeneratesOversizedSessionWithControlZTail) +{ + write("1234567890\r\noriginal-id\r\n" + std::string(1, '\x1a') + + std::string(MAX_FILE_CONTENTS_SIZE, 'x')); + TestLogSessionDataProvider provider(cachePath); + provider.CreateLogSessionData(); + ASSERT_NE(provider.GetLogSessionData(), nullptr); + EXPECT_NE(provider.GetLogSessionData()->getSessionFirstTime(), uint64_t{1234567890}); + EXPECT_NE(provider.GetLogSessionData()->getSessionSDKUid(), "original-id"); + EXPECT_FALSE(provider.GetLogSessionData()->getSessionSDKUid().empty()); + EXPECT_LE(FileGetContents(sessionPath.c_str()).size(), MAX_FILE_CONTENTS_SIZE); +} diff --git a/tests/unittests/PayloadDecoderTests.cpp b/tests/unittests/PayloadDecoderTests.cpp index 0975fbc82..5ec715f54 100644 --- a/tests/unittests/PayloadDecoderTests.cpp +++ b/tests/unittests/PayloadDecoderTests.cpp @@ -6,6 +6,12 @@ #include "PayloadDecoder.hpp" #include "utils/ZlibUtils.hpp" +#if defined(HAVE_MAT_ZLIB) && defined(HAVE_MAT_JSONHPP) +#include "bond/All.hpp" +#include "bond/generated/CsProtocol_writers.hpp" +#include +#endif + using namespace testing; using namespace MAT; @@ -50,6 +56,33 @@ namespace } } +#if defined(HAVE_MAT_ZLIB) && defined(HAVE_MAT_JSONHPP) +TEST(PayloadDecoderTests, DecodeRequest_ManyRecordsPreservesOrderAndContents) +{ + constexpr size_t count = 4096; + std::vector input; + for (size_t i = 0; i < count; ++i) + { + auto record = MakeMinimalRecord(); + record.name = "Event." + std::to_string(i) + std::string(512, 'x'); + std::vector encoded; + bond_lite::CompactBinaryProtocolWriter writer(encoded); + bond_lite::Serialize(writer, record); + input.insert(input.end(), encoded.begin(), encoded.end()); + } + + std::string output; + ASSERT_TRUE(exporters::DecodeRequest(input, output, false)); + const auto records = nlohmann::json::parse(output); + ASSERT_EQ(records.size(), count); + for (size_t i = 0; i < count; ++i) + { + EXPECT_EQ(records[i]["name"].get(), + "Event." + std::to_string(i) + std::string(512, 'x')); + } +} +#endif + // A telemetry event field can legitimately contain bytes that are not valid // UTF-8. nlohmann::json::dump() defaults to error_handler_t::strict, which // throws type_error.316 on such input. Because DecodeRecord/DecodeRequest run diff --git a/tests/unittests/UnitTests.vcxproj b/tests/unittests/UnitTests.vcxproj index 953718f00..ec66e75b6 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -432,6 +432,7 @@ + diff --git a/tests/unittests/bounded-input-tests.cpp b/tests/unittests/bounded-input-tests.cpp index 82ac268ee..852cb09e2 100644 --- a/tests/unittests/bounded-input-tests.cpp +++ b/tests/unittests/bounded-input-tests.cpp @@ -80,6 +80,30 @@ TEST_F(BoundedInputTests, RejectsOversizedFilesInsteadOfParsingAPrefix) EXPECT_TRUE(FileGetContents(path.c_str()).empty()); } +TEST_F(BoundedInputTests, CountsPhysicalCrlfBytesAtTheFileLimit) +{ + std::string exact; + for (size_t i = 0; i < MAX_FILE_CONTENTS_SIZE / 2; ++i) + { + exact += "\r\n"; + } + write(exact); + EXPECT_EQ(FileGetContents(path.c_str()), exact); + write(exact + "x"); + EXPECT_TRUE(FileGetContents(path.c_str()).empty()); +} + +TEST_F(BoundedInputTests, DoesNotTreatControlZAsEndOfFile) +{ + const std::string prefix = "1234567890\r\noriginal-id\r\n"; + const std::string exact = prefix + '\x1a' + + std::string(MAX_FILE_CONTENTS_SIZE - prefix.size() - 1, 'x'); + write(exact); + EXPECT_EQ(FileGetContents(path.c_str()), exact); + write(exact + "x"); + EXPECT_TRUE(FileGetContents(path.c_str()).empty()); +} + TEST_F(BoundedInputTests, RejectsImpossibleContainerCountsBeforeAllocation) { const std::vector input = {bond_lite::BT_STRING, 0xff, 0xff, 0xff, 0xff, 0x0f}; From 30d70f626184ab2b765a7ff65aa98c8bed2ca463 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 14:13:59 -0500 Subject: [PATCH 6/9] Keep session persistence assertions valid with binary file reads Normalize the CRLF terminator in the legacy functional-test reader now that FileGetContents preserves physical bytes. Retain exact UID comparisons instead of relaxing the persistence assertion. Files changed: - tests/functests/LogSessionDataFuncTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- tests/functests/LogSessionDataFuncTests.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/functests/LogSessionDataFuncTests.cpp b/tests/functests/LogSessionDataFuncTests.cpp index 9afde6926..936e9203a 100644 --- a/tests/functests/LogSessionDataFuncTests.cpp +++ b/tests/functests/LogSessionDataFuncTests.cpp @@ -59,6 +59,10 @@ std::string RemoveWhitespace(std::string& str) { trimmedString.erase(trimmedString.length() - 1); } + if (!trimmedString.empty() && trimmedString[trimmedString.length() - 1] == '\r') + { + trimmedString.erase(trimmedString.length() - 1); + } return trimmedString; } From a045bc232cf315ebcbd09c64f660a843eb4fe45d Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 14:38:52 -0500 Subject: [PATCH 7/9] Bound Android headers by encoded bytes before JNI allocation Address Copilot comment 4187949583: UTF-16 character counts undercount non-ASCII headers. Match JNI modified UTF-8 sizes without allocating encoded strings, retaining the exact ASCII limit and aggregate framing. Verified JNI encoding at lib/http/HttpClient_Android.cpp:573-589. Files changed: - lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java - lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java - docs/PAL.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- docs/PAL.md | 8 +++-- .../applications/events/HttpClient.java | 24 ++++++++++--- .../events/HttpClientRequestTest.java | 34 +++++++++++++++++++ 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/docs/PAL.md b/docs/PAL.md index 9d3b9a5f8..916d4ce06 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -102,9 +102,11 @@ byte limit; session parsing accepts both LF and CRLF line endings. The header budget includes framing for native raw headers or a minimum four-byte allowance per name/value pair. Windows native queries measure raw UTF-16/ANSI buffer bytes; WinRT conservatively budgets up to three UTF-8 bytes -per UTF-16 unit. Android bounds Java strings before JNI additionally checks -their encoded byte sizes. OS networking frameworks may have their own -internal limits; the SDK limits its own copies and streaming body reads. +per UTF-16 unit. Android counts JNI modified UTF-8 bytes without allocating +encoded strings, including two bytes for NUL and three per surrogate, before +JNI additionally checks their encoded byte sizes. OS networking frameworks may +have their own internal limits; the SDK limits its own copies and streaming +body reads. Command output is drained after the stored prefix reaches its limit so closing the pipe cannot block behind a child waiting to write to a full pipe. diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java index b310b74a5..49ff90e64 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/HttpClient.java @@ -108,16 +108,15 @@ static byte[] readResponseBody(InputStream input) throws IOException { static String[] readResponseHeaders(Map> headers) throws IOException { Vector result = new Vector<>(); - int size = 0; + int remaining = MAX_HTTP_RESPONSE_HEADERS_SIZE; for (Map.Entry> entry : headers.entrySet()) { if (entry.getKey() != null) { for (String value : entry.getValue()) { - // Pre-bound Java strings before JNI checks their encoded byte sizes. - long count = (long) entry.getKey().length() + value.length() + 4; - if (count > MAX_HTTP_RESPONSE_HEADERS_SIZE - size) { + if (remaining < 4) { throw new IOException("HTTP response headers exceed buffered size limit"); } - size += (int) count; + remaining = remainingHeaderBytes(entry.getKey(), remaining - 4); + remaining = remainingHeaderBytes(value, remaining); result.add(entry.getKey()); result.add(value); } @@ -126,6 +125,21 @@ static String[] readResponseHeaders(Map> headers) throws IO return result.toArray(new String[0]); } + private static int remainingHeaderBytes(String value, int remaining) throws IOException { + if (value.length() > remaining) { + throw new IOException("HTTP response headers exceed buffered size limit"); + } + for (int i = 0; i < value.length(); ++i) { + // Match JNI modified UTF-8: NUL uses two bytes and each surrogate uses three. + char c = value.charAt(i); + remaining -= c != 0 && c <= 0x7f ? 1 : c <= 0x7ff ? 2 : 3; + if (remaining < 0) { + throw new IOException("HTTP response headers exceed buffered size limit"); + } + } + return remaining; + } + Request( @NonNull HttpClient parent, String url, diff --git a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java index 7430ccc06..5bd1d1110 100644 --- a/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java +++ b/lib/android_build/maesdk/src/test/java/com/microsoft/applications/events/HttpClientRequestTest.java @@ -53,6 +53,40 @@ public void rejectsOversizedAggregateResponseHeaders() throws IOException { Request.readResponseHeaders(headers); } + @Test + public void boundsResponseHeadersByJniEncodedBytes() throws IOException { + String[] characters = {"x", "\u0080", "\u07ff", "\u0800", "\ud800", "\0", "\ud83d\ude00"}; + int[] byteSizes = {1, 2, 2, 3, 3, 2, 6}; + String key = "X\u0800"; + int available = Request.MAX_HTTP_RESPONSE_HEADERS_SIZE - 8; + for (int i = 0; i < characters.length; ++i) { + StringBuilder builder = new StringBuilder(); + for (int j = 0; j < available / byteSizes[i]; ++j) { + builder.append(characters[i]); + } + for (int j = 0; j < available % byteSizes[i]; ++j) { + builder.append('x'); + } + String value = builder.toString(); + Map> headers = new HashMap<>(); + headers.put(key, Collections.singletonList(value)); + Assert.assertArrayEquals(new String[] {key, value}, Request.readResponseHeaders(headers)); + headers.put(key, Collections.singletonList(value + "x")); + Assert.assertThrows(IOException.class, () -> Request.readResponseHeaders(headers)); + } + } + + @Test + public void boundsAggregateNonAsciiResponseHeaders() throws IOException { + String value = new String(new char[(Request.MAX_HTTP_RESPONSE_HEADERS_SIZE - 16) / 6]) + .replace('\0', '\u0800'); + Map> headers = new HashMap<>(); + headers.put("X\u0800", java.util.Arrays.asList(value, value)); + Assert.assertEquals(4, Request.readResponseHeaders(headers).length); + headers.put("X\u0800", java.util.Arrays.asList(value, value + "x")); + Assert.assertThrows(IOException.class, () -> Request.readResponseHeaders(headers)); + } + @Test public void testHeaders() { byte[] bytes = "key1value1key33key2value2".getBytes(UTF_8); From a58be385b5249c6bd9905f5c5933aaa98e943275 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 15:03:42 -0500 Subject: [PATCH 8/9] Honor the device-ID opt-out in Android Gradle and WinRT builds Address Copilot comments 4187752458 and 4187752558. Forward the Gradle property through the shared Android configuration so SDK and app native builds cannot silently retain collection. Exercise OFF through the same CI Gradle path and compile the native device-ID regressions in the app. Mark the WinRT constructor parameter unused only in the disabled branch so /W4 /WX builds retain the advertised opt-out without suppressions. Verified both Android CMake caches and compile commands with OFF, built SDK/app arm64 targets with ON and OFF, and compiled the WinRT collector with VS 2026 in enabled Release and disabled Release/Debug configurations. Files changed: - .github/workflows/build-android.yml - docs/PAL.md - lib/android_build/app/src/main/cpp/CMakeLists.txt - lib/android_build/tools.gradle - lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- .github/workflows/build-android.yml | 4 ++++ docs/PAL.md | 6 +++++- lib/android_build/app/src/main/cpp/CMakeLists.txt | 1 + lib/android_build/tools.gradle | 7 +++++++ lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp | 2 ++ 5 files changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-android.yml b/.github/workflows/build-android.yml index 96cb76c80..1e167f762 100644 --- a/.github/workflows/build-android.yml +++ b/.github/workflows/build-android.yml @@ -80,6 +80,10 @@ jobs: run: | .\gradlew.bat maesdk:test working-directory: lib\android_build + - name: Gradle Build without native device-ID collection + run: | + .\gradlew.bat maesdk:assembleDebug app:assembleDebug -PMATSDK_ENABLE_DEVICE_ID=OFF + working-directory: lib\android_build - name: Upload Reports if: failure() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 diff --git a/docs/PAL.md b/docs/PAL.md index 916d4ce06..c49f37312 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -73,7 +73,11 @@ placeholder. Applications can still supply their own ID through `ISemanticContext::SetDeviceId`; registration does not overwrite it with an empty automatically collected ID. Android's Java bridge consults the native build setting before accessing `ANDROID_ID`; use the matching Java bridge -sources with the native SDK. +sources with the native SDK. For Android Gradle builds, pass +`-PMATSDK_ENABLE_DEVICE_ID=OFF` to apply the setting to both the SDK AAR and +the test application. The Gradle property takes precedence over the +`MATSDK_ENABLE_DEVICE_ID` environment variable; both default to `ON` when +unspecified. This option does not disable session/SDK identifiers or control device IDs added independently by the operating system's UTC telemetry pipeline. diff --git a/lib/android_build/app/src/main/cpp/CMakeLists.txt b/lib/android_build/app/src/main/cpp/CMakeLists.txt index 64e93ab16..24d093150 100644 --- a/lib/android_build/app/src/main/cpp/CMakeLists.txt +++ b/lib/android_build/app/src/main/cpp/CMakeLists.txt @@ -31,6 +31,7 @@ set(TESTS_SRCS ${SDK_ROOT}/tests/unittests/DataViewerCollectionTests.cpp ${SDK_ROOT}/tests/unittests/DebugEventSourceTests.cpp ${SDK_ROOT}/tests/unittests/DeviceStateHandlerTests.cpp + ${SDK_ROOT}/tests/unittests/device-id-tests.cpp ${SDK_ROOT}/tests/unittests/DiskLocalStorageTests.cpp ${SDK_ROOT}/tests/unittests/EventFilterCollectionTests.cpp ${SDK_ROOT}/tests/unittests/EventPropertiesStorageTests.cpp diff --git a/lib/android_build/tools.gradle b/lib/android_build/tools.gradle index 7f4d1d743..957ce9212 100644 --- a/lib/android_build/tools.gradle +++ b/lib/android_build/tools.gradle @@ -6,6 +6,13 @@ android { defaultConfig { minSdk 23 targetSdk 34 + externalNativeBuild { + cmake { + arguments "-DMATSDK_ENABLE_DEVICE_ID=" + + (project.findProperty("MATSDK_ENABLE_DEVICE_ID") ?: + System.getenv("MATSDK_ENABLE_DEVICE_ID") ?: "ON") + } + } } } diff --git a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp index e683a6d3a..99f7d9ec4 100644 --- a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp @@ -87,6 +87,8 @@ namespace PAL_NS_BEGIN { // Workaround for Windows OS bug VSO: 11314171 - sometimes NetworkInformation triggers exception m_device_id = DEFAULT_DEVICE_ID; } +#else + (void)configuration; #endif #ifdef _WIN32_WINNT_WIN10 From fd72314b142a92496f6a0f8b4a08ef06e7dcde26 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 5 Oct 2026 15:53:18 -0500 Subject: [PATCH 9/9] Limit new bounds to targeted external-input safeguards Remove blanket metadata, OS-buffer and session-file limits to preserve existing values and persisted identities rather than truncate or reject unlikely outliers. Keep NUL-aware bounded command-line reads, HTTP buffering limits and decompression safeguards. Reject impossible binary counts based on the actual input, not an arbitrary element/record cap. Replace cap-enforcement tests with exact preservation regressions for large metadata, session identifiers and valid containers. Retain the device-ID opt-out and existing platform text-read behavior. Files changed: - docs/CsProtocol-decoding.md - docs/PAL.md - lib/bond/CompactBinaryProtocolReader.hpp - lib/decoder/PayloadDecoder.cpp - lib/offline/LogSessionDataProvider.cpp - lib/pal/PAL.cpp - lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp - lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp - lib/pal/posix/DeviceInformationImpl_Android.cpp - lib/pal/posix/SystemInformationImpl_Android.cpp - lib/pal/posix/sysinfo_sources.cpp - lib/pal/posix/sysinfo_sources.hpp - lib/pal/posix/sysinfo_utils_ios.mm - lib/pal/posix/sysinfo_utils_mac.mm - lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp - lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp - lib/shared/PlatformHelpers.h - lib/utils/FileUtils.cpp - lib/utils/FileUtils.hpp - lib/utils/Utils.cpp - lib/utils/Utils.hpp - tests/unittests/LogSessionDataTests.cpp - tests/unittests/bounded-input-tests.cpp - tests/unittests/device-id-tests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 95c41974-5533-4eec-89fb-6885edb2c3a5 --- docs/CsProtocol-decoding.md | 8 +- docs/PAL.md | 22 ++- lib/bond/CompactBinaryProtocolReader.hpp | 4 - lib/decoder/PayloadDecoder.cpp | 5 - lib/offline/LogSessionDataProvider.cpp | 7 +- lib/pal/PAL.cpp | 28 ++-- .../WindowsDesktopDeviceInformationImpl.cpp | 8 +- .../WindowsDesktopSystemInformationImpl.cpp | 6 - .../posix/DeviceInformationImpl_Android.cpp | 17 +-- .../posix/SystemInformationImpl_Android.cpp | 16 +-- lib/pal/posix/sysinfo_sources.cpp | 50 +++---- lib/pal/posix/sysinfo_sources.hpp | 5 +- lib/pal/posix/sysinfo_utils_ios.mm | 11 +- lib/pal/posix/sysinfo_utils_mac.mm | 7 +- .../WindowsRuntimeDeviceInformationImpl.cpp | 4 +- .../WindowsRuntimeSystemInformationImpl.cpp | 10 +- lib/shared/PlatformHelpers.h | 7 - lib/utils/FileUtils.cpp | 29 ++-- lib/utils/FileUtils.hpp | 2 - lib/utils/Utils.cpp | 46 ------ lib/utils/Utils.hpp | 10 -- tests/unittests/LogSessionDataTests.cpp | 27 ++-- tests/unittests/bounded-input-tests.cpp | 132 ++++++++---------- tests/unittests/device-id-tests.cpp | 7 +- 24 files changed, 170 insertions(+), 298 deletions(-) diff --git a/docs/CsProtocol-decoding.md b/docs/CsProtocol-decoding.md index 9a8fd1ca6..1737795ad 100644 --- a/docs/CsProtocol-decoding.md +++ b/docs/CsProtocol-decoding.md @@ -36,10 +36,10 @@ compressed input and the inflated output. Inflation uses a fixed-size work buffer and stops before growing the output beyond that limit; it does not estimate output size from a presumed compression ratio. -Binary container counts are limited to 65,536 elements and must fit the -remaining encoded input (at least one byte per sequence element or two bytes -per map entry). Invalid counts are rejected before generated readers allocate -their containers. Requests are also limited to 65,536 decoded records. +Binary container counts must fit the remaining encoded input (at least one +byte per sequence element or two bytes per map entry). Impossible counts are +rejected before generated readers allocate their containers; no separate +fixed element-count or record-count limit is imposed. Record-boundary scanning reads the request in place and copies only each individual record, so scanning and input copying are linear in request size. Decoding failure returns `false` rather than reporting a partial request as diff --git a/docs/PAL.md b/docs/PAL.md index c49f37312..ba4cae4fd 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -82,27 +82,24 @@ unspecified. This option does not disable session/SDK identifiers or control device IDs added independently by the operating system's UTC telemetry pipeline. -## Input size limits +## Targeted input safeguards -Automatically collected system, device, application, and network-provider -strings are limited to 4 KiB of UTF-8, without splitting a UTF-8 sequence. -This does not limit values explicitly supplied by an application through -event properties or semantic context. +The SDK bounds the command-line input involved in initialization and buffered +HTTP responses. It does not impose a blanket metadata limit on system, +device, application, or network-provider strings, additional limits on +OS-reported buffer sizes, or a size limit on persisted session files. | Input | Limit | Oversize behavior | | --- | --- | --- | | POSIX application identifier | 4 KiB, stopping at the first NUL in `/proc/self/cmdline` | Truncate the executable name; never collect arguments or run a regex | -| POSIX OS release file and device-ID command output | 64 KiB | Bound the collected prefix and log a warning | -| OS-sized executable path and device-model buffers | 64 KiB | Reject before allocating | -| Windows version-resource and adapter-information buffers | 1 MiB | Reject before allocating; retain the existing missing-information fallback | -| Session sidecar file read through `FileGetContents` | 4 KiB of physical file bytes | Reject the whole file, log a warning, and regenerate session data | | HTTP response body | 16 MiB | Fail the request rather than retain an oversized response | | HTTP response headers | 64 KiB | Fail the request rather than retain oversized headers | POSIX OS release values use exact line-key matching rather than recursive regular expressions. Curl response headers are also parsed without regex. -Session files are read in binary mode so CRLF and Ctrl+Z cannot bypass the -byte limit; session parsing accepts both LF and CRLF line endings. +Command-line truncation preserves UTF-8 boundaries. Other metadata is not +truncated by these safeguards. Session files retain the existing platform +text-read behavior; session parsing accepts both LF and CRLF line endings. The header budget includes framing for native raw headers or a minimum four-byte allowance per name/value pair. Windows native queries measure raw UTF-16/ANSI buffer bytes; WinRT conservatively budgets up to three UTF-8 bytes @@ -112,9 +109,6 @@ JNI additionally checks their encoded byte sizes. OS networking frameworks may have their own internal limits; the SDK limits its own copies and streaming body reads. -Command output is drained after the stored prefix reaches its limit so closing -the pipe cannot block behind a child waiting to write to a full pipe. - Caller-provided events retain the existing configured serialized-event, upload, and offline-cache size policies; no new per-property limit is applied. See [decoder limits](CsProtocol-decoding.md#decoder-input-limits) for the diff --git a/lib/bond/CompactBinaryProtocolReader.hpp b/lib/bond/CompactBinaryProtocolReader.hpp index a1502a8eb..0dc336bae 100644 --- a/lib/bond/CompactBinaryProtocolReader.hpp +++ b/lib/bond/CompactBinaryProtocolReader.hpp @@ -26,8 +26,6 @@ class CompactBinaryProtocolReader { size_t m_ofs; public: - static constexpr uint32_t MAX_CONTAINER_ELEMENTS = 64 * 1024; - CompactBinaryProtocolReader(std::vector const& input) : m_input(input), m_ofs(0) @@ -221,7 +219,6 @@ class CompactBinaryProtocolReader { elementType = (raw & 31); return ReadUInt32(size) && - size <= MAX_CONTAINER_ELEMENTS && size <= m_input.size() - m_ofs; } @@ -230,7 +227,6 @@ class CompactBinaryProtocolReader { return ReadUInt8(keyType) && (keyType >> 5 == 0) && ReadUInt8(valueType) && (valueType >> 5 == 0) && ReadUInt32(size) && - size <= MAX_CONTAINER_ELEMENTS && size <= (m_input.size() - m_ofs) / 2; } diff --git a/lib/decoder/PayloadDecoder.cpp b/lib/decoder/PayloadDecoder.cpp index 22c788c5d..5c8ad7adf 100644 --- a/lib/decoder/PayloadDecoder.cpp +++ b/lib/decoder/PayloadDecoder.cpp @@ -71,11 +71,6 @@ namespace clienttelemetry { size_t length = 0; while (i < request.size()) { - if (v.size() >= bond_lite::CompactBinaryProtocolReader::MAX_CONTAINER_ELEMENTS) - { - TEST_LOG_ERROR("Too many records in decoder input"); - return {}; - } Record result; length = request.size() - i; size_t j = 3; diff --git a/lib/offline/LogSessionDataProvider.cpp b/lib/offline/LogSessionDataProvider.cpp index a18fd4455..f4885eff3 100644 --- a/lib/offline/LogSessionDataProvider.cpp +++ b/lib/offline/LogSessionDataProvider.cpp @@ -142,17 +142,12 @@ namespace MAT_NS_BEGIN uint64_t& sessionFirstTimeLaunch, std::string& sessionSDKUid) { - if (content.size() > MAX_FILE_CONTENTS_SIZE) - { - LOG_WARN("Session data exceeds %zu bytes; rejecting", MAX_FILE_CONTENTS_SIZE); - return false; - } if (content.empty()) { return false; } std::vector v; StringUtils::SplitString(content, '\n', v); - if (v.size() != 3 || !v[2].empty()) { + if (v.size() != 3) { return false; } remove_eol(v[0]); diff --git a/lib/pal/PAL.cpp b/lib/pal/PAL.cpp index 3fc1fad0f..59873c282 100644 --- a/lib/pal/PAL.cpp +++ b/lib/pal/PAL.cpp @@ -572,35 +572,35 @@ namespace PAL_NS_BEGIN { #ifdef MATSDK_DISABLE_DEVICE_ID if (!deviceId.empty()) #endif - context->SetDeviceId(MAT::boundedSystemInfo(deviceId.c_str())); - context->SetDeviceModel(MAT::boundedSystemInfo(m_DeviceInformation->GetModel().c_str())); - context->SetDeviceMake(MAT::boundedSystemInfo(m_DeviceInformation->GetManufacturer().c_str())); + context->SetDeviceId(deviceId); + context->SetDeviceModel(m_DeviceInformation->GetModel()); + context->SetDeviceMake(m_DeviceInformation->GetManufacturer()); } if (m_SystemInformation != nullptr) { // Get SystemInfo common fields - context->SetOsVersion(MAT::boundedSystemInfo(m_SystemInformation->GetOsMajorVersion().c_str())); - context->SetOsName(MAT::boundedSystemInfo(m_SystemInformation->GetOsName().c_str())); - context->SetOsBuild(MAT::boundedSystemInfo(m_SystemInformation->GetOsFullVersion().c_str())); - context->SetDeviceClass(MAT::boundedSystemInfo(m_SystemInformation->GetDeviceClass().c_str())); + context->SetOsVersion(m_SystemInformation->GetOsMajorVersion()); + context->SetOsName(m_SystemInformation->GetOsName()); + context->SetOsBuild(m_SystemInformation->GetOsFullVersion()); + context->SetDeviceClass(m_SystemInformation->GetDeviceClass()); // AppInfo fields - context->SetAppId(MAT::boundedSystemInfo(m_SystemInformation->GetAppId().c_str())); - context->SetAppVersion(MAT::boundedSystemInfo(m_SystemInformation->GetAppVersion().c_str())); - context->SetAppLanguage(MAT::boundedSystemInfo(m_SystemInformation->GetAppLanguage().c_str())); + context->SetAppId(m_SystemInformation->GetAppId()); + context->SetAppVersion(m_SystemInformation->GetAppVersion()); + context->SetAppLanguage(m_SystemInformation->GetAppLanguage()); // UserInfo fields. - context->SetUserLanguage(MAT::boundedSystemInfo(m_SystemInformation->GetUserLanguage().c_str())); - context->SetUserTimeZone(MAT::boundedSystemInfo(m_SystemInformation->GetUserTimeZone().c_str())); + context->SetUserLanguage(m_SystemInformation->GetUserLanguage()); + context->SetUserTimeZone(m_SystemInformation->GetUserTimeZone()); //context->SetUserAdvertisingId(m_SystemInformation->GetUserAdvertisingId()); - context->SetCommercialId(MAT::boundedSystemInfo(m_SystemInformation->GetCommercialId().c_str())); + context->SetCommercialId(m_SystemInformation->GetCommercialId()); } if (m_NetworkInformation != nullptr) { // Get NetworkInfo common fields - context->SetNetworkProvider(MAT::boundedSystemInfo(m_NetworkInformation->GetNetworkProvider().c_str())); + context->SetNetworkProvider(m_NetworkInformation->GetNetworkProvider()); context->SetNetworkCost(m_NetworkInformation->GetNetworkCost()); context->SetNetworkType(m_NetworkInformation->GetNetworkType()); } diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index d0b539497..79a815fad 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -9,7 +9,6 @@ #include "pal/PAL.hpp" #include "pal/DeviceInformationImpl.hpp" #include "utils/StringUtils.hpp" -#include "utils/Utils.hpp" #include "WindowsEnvironmentInfo.hpp" MATSDK_LOG_INST_COMPONENT_NS("DeviceInfo", "Win32 Desktop Device Information") @@ -67,16 +66,11 @@ namespace PAL_NS_BEGIN { * the ulOutBufLen parameter contains the required buffer size, so we retry with * suggested value of ulOutBufLen, assuming it is bigger than the default. */ - if (ulOutBufLen > sizeof(IP_ADAPTER_INFO) && - ulOutBufLen <= MAX_SYSTEM_INFO_BLOB_SIZE) + if (ulOutBufLen > sizeof(IP_ADAPTER_INFO)) { FREE(pAdapterInfo); goto retry_bigger_buffer; } - if (ulOutBufLen > MAX_SYSTEM_INFO_BLOB_SIZE) - { - LOG_WARN("Adapter information exceeds %zu bytes; rejecting", MAX_SYSTEM_INFO_BLOB_SIZE); - } } if ((result == ERROR_SUCCESS) && (pAdapterInfo->AdapterName[0] != 0)) { diff --git a/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp index d5f62651e..17eea75df 100644 --- a/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp @@ -120,12 +120,6 @@ namespace PAL_NS_BEGIN { { return{}; } - if (dwVersionInfoSize > MAX_SYSTEM_INFO_BLOB_SIZE) - { - LOG_WARN("Executable version resource exceeds %zu bytes; rejecting", MAX_SYSTEM_INFO_BLOB_SIZE); - return {}; - } - buffer.resize(dwVersionInfoSize); if (GetFileVersionInfo(applicationFullPath.data(), 0, dwVersionInfoSize, &buffer[0]) == 0) diff --git a/lib/pal/posix/DeviceInformationImpl_Android.cpp b/lib/pal/posix/DeviceInformationImpl_Android.cpp index 1a249c92c..0f4036599 100644 --- a/lib/pal/posix/DeviceInformationImpl_Android.cpp +++ b/lib/pal/posix/DeviceInformationImpl_Android.cpp @@ -5,7 +5,6 @@ #define LOG_MODULE DBG_API #include "pal/PAL.hpp" #include "pal/DeviceInformationImpl.hpp" -#include "utils/Utils.hpp" #include #include @@ -204,18 +203,17 @@ namespace PAL_NS_BEGIN { jStr = pEnv->GetStringUTFChars(androidIdJstr, &isCopy); std::string deviceId("a:"); - deviceId += MAT::boundedSystemInfo(jStr); - deviceId = MAT::boundedSystemInfo(deviceId.c_str()); + deviceId += jStr; pEnv->ReleaseStringUTFChars(androidIdJstr, jStr); AndroidDeviceInformationConnector::setDeviceId(std::move(deviceId)); #endif jStr = pEnv->GetStringUTFChars(manufacturerJstr, &isCopy); - std::string manufacturer = MAT::boundedSystemInfo(jStr); + std::string manufacturer = jStr; pEnv->ReleaseStringUTFChars(manufacturerJstr, jStr); jStr = pEnv->GetStringUTFChars(modelJstr, &isCopy); - std::string model = MAT::boundedSystemInfo(jStr); + std::string model = jStr; pEnv->ReleaseStringUTFChars(modelJstr, jStr); AndroidDeviceInformationConnector::setManufacturer(std::move(manufacturer)); @@ -267,14 +265,17 @@ Java_com_microsoft_applications_events_HttpClient_onPowerChange(JNIEnv* env, ) { auto start = env->GetStringUTFChars(id, nullptr); - PAL::AndroidDeviceInformationConnector::setDeviceId(MAT::boundedSystemInfo(start)); + auto end = start + env->GetStringUTFLength(id); + PAL::AndroidDeviceInformationConnector::setDeviceId(std::string(start, end)); env->ReleaseStringUTFChars(id, start); start = env->GetStringUTFChars(manufacturer, nullptr); - PAL::AndroidDeviceInformationConnector::setManufacturer(MAT::boundedSystemInfo(start)); + end = start + env->GetStringUTFLength(manufacturer); + PAL::AndroidDeviceInformationConnector::setManufacturer(std::string(start, end)); env->ReleaseStringUTFChars(manufacturer, start); start = env->GetStringUTFChars(model, nullptr); - PAL::AndroidDeviceInformationConnector::setModel(MAT::boundedSystemInfo(start)); + end = start + env->GetStringUTFLength(model); + PAL::AndroidDeviceInformationConnector::setModel(std::string(start, end)); env->ReleaseStringUTFChars(model, start); } diff --git a/lib/pal/posix/SystemInformationImpl_Android.cpp b/lib/pal/posix/SystemInformationImpl_Android.cpp index 25687ea9c..ed62bd259 100644 --- a/lib/pal/posix/SystemInformationImpl_Android.cpp +++ b/lib/pal/posix/SystemInformationImpl_Android.cpp @@ -6,7 +6,6 @@ #include #include "pal/PAL.hpp" #include "pal/SystemInformationImpl.hpp" -#include "utils/Utils.hpp" #include @@ -29,7 +28,8 @@ namespace PAL_NS_BEGIN { static void setValue(JNIEnv *env, std::string & s, jstring js) { const char *start = env->GetStringUTFChars(js, nullptr); - s = MAT::boundedSystemInfo(start); + auto end = start + env->GetStringUTFLength(js); + s = std::string(start, end); env->ReleaseStringUTFChars(js, start); } @@ -102,7 +102,7 @@ namespace PAL_NS_BEGIN { jstring packageNameJstr = reinterpret_cast(pEnv->CallObjectMethod(activity, getPackageNameMid)); jStr = pEnv->GetStringUTFChars(packageNameJstr, &isCopy); - std::string packageName = MAT::boundedSystemInfo(jStr); + std::string packageName = jStr; pEnv->ReleaseStringUTFChars(packageNameJstr, jStr); std::string versionName; @@ -114,7 +114,7 @@ namespace PAL_NS_BEGIN { pEnv->GetObjectField(packageInfo, versionNameFid)); jStr = pEnv->GetStringUTFChars(versionNameJstr, &isCopy); - versionName = MAT::boundedSystemInfo(jStr); + versionName = jStr; pEnv->ReleaseStringUTFChars(versionNameJstr, jStr); } pEnv->ExceptionClear(); @@ -122,7 +122,7 @@ namespace PAL_NS_BEGIN { jobject defaultLocale = pEnv->CallStaticObjectMethod(localeClass, getDefaultLocaleMid); jstring launguageTagJstr = reinterpret_cast(pEnv->CallObjectMethod(defaultLocale, toLanguageTagMid)); jStr = pEnv->GetStringUTFChars(launguageTagJstr, &isCopy); - std::string languageTag = MAT::boundedSystemInfo(jStr); + std::string languageTag = jStr; pEnv->ReleaseStringUTFChars(launguageTagJstr, jStr); if (SDK_INT < 21) { @@ -131,19 +131,19 @@ namespace PAL_NS_BEGIN { jstring versionReleaseJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildVersionClass, releaseFid)); jStr = pEnv->GetStringUTFChars(versionReleaseJstr, &isCopy); - std::string versionRelease = MAT::boundedSystemInfo(jStr); + std::string versionRelease = jStr; pEnv->ReleaseStringUTFChars(versionReleaseJstr, jStr); jstring versionIncrementalJstr = reinterpret_cast(pEnv->GetStaticObjectField(buildVersionClass, incrementalFid)); jStr = pEnv->GetStringUTFChars(versionIncrementalJstr, &isCopy); - std::string versionIncremental = MAT::boundedSystemInfo(jStr); + std::string versionIncremental = jStr; pEnv->ReleaseStringUTFChars(versionIncrementalJstr, jStr); std::string osVersion = versionRelease + " " + versionIncremental; AndroidSystemInformationConnector::s_app_id = std::move(packageName); AndroidSystemInformationConnector::s_app_language = std::move(languageTag); AndroidSystemInformationConnector::s_app_version = std::move(versionName); - AndroidSystemInformationConnector::s_os_full_version = MAT::boundedSystemInfo(osVersion.c_str()); + AndroidSystemInformationConnector::s_os_full_version = std::move(osVersion); AndroidSystemInformationConnector::s_os_major_version = std::move(versionRelease); AndroidSystemInformationConnector::s_device_class = (std::move(device_class)); } diff --git a/lib/pal/posix/sysinfo_sources.cpp b/lib/pal/posix/sysinfo_sources.cpp index 64a5b9ce5..14ddcf0aa 100644 --- a/lib/pal/posix/sysinfo_sources.cpp +++ b/lib/pal/posix/sysinfo_sources.cpp @@ -10,7 +10,6 @@ */ #include "sysinfo_sources_impl.hpp" -#include "utils/Utils.hpp" #include "pal/PAL.hpp" #include @@ -78,9 +77,9 @@ std::string get_app_name() uint32_t length = static_cast(appId.size()); if(_NSGetExecutablePath(&appId[0], &length)) { - if (length == 0 || length > MAT::MAX_SYSTEM_INFO_SOURCE_SIZE) + if (length == 0) { - LOG_WARN("Executable path exceeds system information source limit"); + LOG_WARN("Executable path length is empty"); return {}; } appId.resize(length, 0); @@ -104,29 +103,30 @@ std::string get_app_name() inline std::string ReadFile(const char *filename, sysinfo_selector selector) { using PAL::getMATSDKLogComponent; - std::ifstream input(filename, std::ios::binary); + auto mode = std::ios::in; + if (selector == sysinfo_selector::first_null) + mode |= std::ios::binary; + std::ifstream input(filename, mode); std::string result; - const size_t limit = selector == sysinfo_selector::key_value - ? MAT::MAX_SYSTEM_INFO_SOURCE_SIZE : MAT::MAX_SYSTEM_INFO_VALUE_SIZE; char ch; - while (result.size() < limit && input.get(ch)) + while (input.get(ch)) { if ((selector == sysinfo_selector::first_null && ch == '\0') || (selector == sysinfo_selector::first_line && ch == '\n')) { return result; } - result.push_back(ch); - } - if (input.get(ch)) - { - if ((selector == sysinfo_selector::first_null && ch == '\0') || - (selector == sysinfo_selector::first_line && ch == '\n')) + if (selector == sysinfo_selector::first_null && result.size() == MAX_COMMAND_LINE_SIZE) { + LOG_WARN("Command-line executable exceeds %zu bytes; truncating", MAX_COMMAND_LINE_SIZE); + result.push_back(ch); + size_t length = MAX_COMMAND_LINE_SIZE; + // Avoid cutting a UTF-8 sequence at the command-line boundary. + while (length > 0 && (static_cast(result[length]) & 0xc0) == 0x80) + --length; + result.resize(length); return result; } - LOG_WARN("System information source exceeds %zu bytes; truncating", limit); - // The extra byte lets boundedSystemInfo preserve UTF-8 boundaries. result.push_back(ch); } if (input.bad()) @@ -156,26 +156,16 @@ static std::string Exec(const char* cmd) return result; } - bool truncated = false; while (fgets(buffer.data(), static_cast(buffer.size()), pipe.get()) != nullptr) { const size_t count = strlen(buffer.data()); - const size_t remaining = MAT::MAX_SYSTEM_INFO_SOURCE_SIZE - result.size(); - result.append(buffer.data(), std::min(count, remaining)); - truncated = truncated || count > remaining; - // Drain the pipe even after reaching the cap so pclose cannot deadlock - // waiting for a child blocked on a full stdout pipe. + result.append(buffer.data(), count); } if (ferror(pipe.get())) { LOG_WARN("Unable to read system information command output"); return {}; } - if (truncated) - { - LOG_WARN("System information command output exceeds %zu bytes; truncating", MAT::MAX_SYSTEM_INFO_SOURCE_SIZE); - } - // Remove EOL. In all use-cases below we don't need it. if (!result.empty() && result[result.length()-1]=='\n') { @@ -187,7 +177,7 @@ static std::string Exec(const char* cmd) #endif /** - * Read a bounded node value, select it without regex and store it in cache + * Read a node value, select it without regex and store it in cache * * @param key Field name * @return true if field value is found and saved in cache @@ -208,7 +198,7 @@ bool sysinfo_sources::fetch(std::string key) std::string contents = ReadFile(kv.second.path, kv.second.selector); if (kv.second.selector != sysinfo_selector::key_value) { - cache[key] = MAT::boundedSystemInfo(contents.c_str()); + cache[key] = std::move(contents); return true; } std::istringstream lines(contents); @@ -227,7 +217,7 @@ bool sysinfo_sources::fetch(std::string key) { value = value.substr(1, value.size() - 2); } - cache[key] = MAT::boundedSystemInfo(value.c_str()); + cache[key] = std::move(value); return true; } } @@ -264,8 +254,6 @@ const std::string& sysinfo_sources::get(std::string key) { if(cache.find(key) == cache.end()) fetch(key); - if (cache[key].size() > MAT::MAX_SYSTEM_INFO_VALUE_SIZE) - cache[key] = MAT::boundedSystemInfo(cache[key].c_str()); return cache[key]; } diff --git a/lib/pal/posix/sysinfo_sources.hpp b/lib/pal/posix/sysinfo_sources.hpp index 041e98440..035f027c4 100644 --- a/lib/pal/posix/sysinfo_sources.hpp +++ b/lib/pal/posix/sysinfo_sources.hpp @@ -7,6 +7,9 @@ #include #include +#include + +static constexpr std::size_t MAX_COMMAND_LINE_SIZE = 4096; /** * System information source path and selector @@ -43,7 +46,7 @@ class sysinfo_sources : public std::multimap { std::map cache; /** - * Read a bounded node value, select it without regex and store it in cache + * Read a node value, select it without regex and store it in cache * * @param key Field name * @return true if field value is found and saved in cache diff --git a/lib/pal/posix/sysinfo_utils_ios.mm b/lib/pal/posix/sysinfo_utils_ios.mm index 5bc446e13..e5fcebc30 100644 --- a/lib/pal/posix/sysinfo_utils_ios.mm +++ b/lib/pal/posix/sysinfo_utils_ios.mm @@ -4,7 +4,6 @@ // #include "sysinfo_utils_apple.hpp" -#include "utils/Utils.hpp" #import #include #import @@ -35,13 +34,13 @@ NSString* modelId = NSProcessInfo.processInfo.environment[@"SIMULATOR_MODEL_IDENTIFIER"]; if (modelId.length > 0) { - return MAT::boundedSystemInfo([modelId UTF8String]); + return std::string([modelId UTF8String]); } NSString* fallbackModel = [[UIDevice currentDevice] model]; if (fallbackModel.length > 0) { - return MAT::boundedSystemInfo([fallbackModel UTF8String]); + return std::string([fallbackModel UTF8String]); } return {}; @@ -51,7 +50,7 @@ if (uname(&systemInfo) < 0) { // Fallback to UIDevice in case of error - deviceModel = MAT::boundedSystemInfo([[[UIDevice currentDevice] model] UTF8String]); + deviceModel = [[[UIDevice currentDevice] model] UTF8String]; } else { @@ -97,14 +96,14 @@ { // Previous implementation pointed to "ProductVersion" on SystemVersion.plist, returning version string in format .. // systemVersion returns string in this same format - return MAT::boundedSystemInfo([[[UIDevice currentDevice] systemVersion] UTF8String]); + return std::string { [[[UIDevice currentDevice] systemVersion] UTF8String] }; } std::string GetDeviceOsRelease() { // Previous implementation pointed to "ProductUserVisibleVersion" on SystemVersion.plist, returning version string in format .. // systemVersion returns string in this same format - return MAT::boundedSystemInfo([[[UIDevice currentDevice] systemVersion] UTF8String]); + return std::string { [[[UIDevice currentDevice] systemVersion] UTF8String] }; } std::string GetDeviceClass() { diff --git a/lib/pal/posix/sysinfo_utils_mac.mm b/lib/pal/posix/sysinfo_utils_mac.mm index 0134dbccf..60c1cfa68 100644 --- a/lib/pal/posix/sysinfo_utils_mac.mm +++ b/lib/pal/posix/sysinfo_utils_mac.mm @@ -5,7 +5,6 @@ #include "sysinfo_utils_apple.hpp" #include "pal/PAL.hpp" -#include "utils/Utils.hpp" #include #include #ifndef MATSDK_DISABLE_DEVICE_ID @@ -25,15 +24,15 @@ std::string deviceModel { }; if (sysctlbyname(query, nullptr, &size, nullptr, 0) == 0) { - if (size == 0 || size > MAT::MAX_SYSTEM_INFO_SOURCE_SIZE) + if (size == 0) { - LOG_WARN("Device model exceeds system information source limit or is empty"); + LOG_WARN("Device model length is empty"); return {}; } deviceModelBuffer.resize(size + 1, '\0'); if (sysctlbyname(query, deviceModelBuffer.data(), &size, nullptr, 0) == 0) { - deviceModel = MAT::boundedSystemInfo(deviceModelBuffer.data()); + deviceModel = deviceModelBuffer.data(); } } diff --git a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp index 99f7d9ec4..5ab652bd7 100644 --- a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp @@ -55,8 +55,8 @@ namespace PAL_NS_BEGIN { m_os_architecture = WindowsEnvironmentInfo::GetProcessorArchitecture(); auto easClientDeviceInformation = ref new ::Windows::Security::ExchangeActiveSyncProvisioning::EasClientDeviceInformation(); - m_model = BoundedFromPlatformString(easClientDeviceInformation->SystemProductName); - m_manufacturer = BoundedFromPlatformString(easClientDeviceInformation->SystemManufacturer); + m_model = FromPlatformString(easClientDeviceInformation->SystemProductName); + m_manufacturer = FromPlatformString(easClientDeviceInformation->SystemManufacturer); #ifndef MATSDK_DISABLE_DEVICE_ID bool isNetDetectEnabled = configuration[CFG_BOOL_ENABLE_NET_DETECT]; diff --git a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp index 39075c210..fa7ac6575 100644 --- a/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeSystemInformationImpl.cpp @@ -38,7 +38,7 @@ namespace PAL_NS_BEGIN { { auto version = Package::Current->Id->Version; - m_app_id = BoundedFromPlatformString(Package::Current->Id->Name); + m_app_id = FromPlatformString(Package::Current->Id->Name); m_app_version = std::to_string(version.Major) + "." + std::to_string(version.Minor) + "." + std::to_string(version.Build) + "." + std::to_string(version.Revision); } else @@ -49,7 +49,7 @@ namespace PAL_NS_BEGIN { m_user_language = "en"; try { if (GlobalizationPreferences::Languages->Size) - m_user_language = BoundedFromPlatformString(GlobalizationPreferences::Languages->GetAt(0)); + m_user_language = FromPlatformString(GlobalizationPreferences::Languages->GetAt(0)); m_user_timezone = WindowsEnvironmentInfo::GetTimeZone(); } catch (AccessDeniedException^) @@ -62,7 +62,7 @@ namespace PAL_NS_BEGIN { try { - m_user_advertising_id = BoundedFromPlatformString(AdvertisingManager::AdvertisingId); + m_user_advertising_id = FromPlatformString(AdvertisingManager::AdvertisingId); } catch (Exception^) { @@ -105,7 +105,7 @@ namespace PAL_NS_BEGIN { m_os_major_version = "10.0"; } - if (BoundedFromPlatformString(AnalyticsInfo::VersionInfo->DeviceFamily) == DeviceFamily_Mobile) + if (FromPlatformString(AnalyticsInfo::VersionInfo->DeviceFamily) == DeviceFamily_Mobile) { m_os_name = WindowsPhoneOSName; } @@ -123,7 +123,7 @@ namespace PAL_NS_BEGIN { catch (...) { //CTDEBUGLOG("Language detection may not be reliable!"); } - m_app_language = BoundedFromPlatformString(primaryLanguage); + m_app_language = FromPlatformString(primaryLanguage); // CTDEBUGLOG("m_app_language=%s", m_app_language.c_str()); } diff --git a/lib/shared/PlatformHelpers.h b/lib/shared/PlatformHelpers.h index ff792a9a2..222ee0024 100644 --- a/lib/shared/PlatformHelpers.h +++ b/lib/shared/PlatformHelpers.h @@ -225,13 +225,6 @@ namespace Microsoft { return std::string(); } -#ifdef _WINRT_DLL - inline std::string BoundedFromPlatformString(String^ platformString) - { - return MAT::boundedSystemInfo(platformString == nullptr ? nullptr : platformString->Data()); - } -#endif - inline String^ ToPlatformString(const std::string& string) { return platform_new String(UTF8ToWstring(string).c_str()); diff --git a/lib/utils/FileUtils.cpp b/lib/utils/FileUtils.cpp index 60fb78de1..fa43f37aa 100644 --- a/lib/utils/FileUtils.cpp +++ b/lib/utils/FileUtils.cpp @@ -97,26 +97,19 @@ namespace MAT_NS_BEGIN * Read file contents into std::string. * * @param UTF-8 flename - * @return File contents up to MAX_FILE_CONTENTS_SIZE bytes, or empty on - * read failure/oversize. Supports UTF-8 or ASCII text files. + * @return File contents. Supports only UTF-8 or ASCII-encoded text files. */ std::string FileGetContents(const char *filename) { +#ifdef _WIN32 + char buff[256] = { 0 }; std::string result; - FILE* fp = FileOpen(filename, "rb"); + FILE* fp = FileOpen(filename, "r"); if (fp != nullptr) { - char buff[256]; - size_t count; - while ((count = fread(buff, 1, sizeof(buff), fp)) != 0) + while (fgets(buff, sizeof(buff), fp)) { - if (count > MAX_FILE_CONTENTS_SIZE - result.size()) - { - LOG_WARN("File contents exceed %zu bytes; rejecting", MAX_FILE_CONTENTS_SIZE); - FileClose(fp); - return {}; - } - result.append(buff, count); + result += buff; } if (ferror(fp)) { @@ -126,6 +119,16 @@ namespace MAT_NS_BEGIN FileClose(fp); } return result; +#else + std::ifstream input(filename); + std::string result((std::istreambuf_iterator(input)), std::istreambuf_iterator()); + if (input.bad()) + { + LOG_WARN("Unable to read file contents"); + return {}; + } + return result; +#endif } /** diff --git a/lib/utils/FileUtils.hpp b/lib/utils/FileUtils.hpp index 61b22a560..620eba4be 100644 --- a/lib/utils/FileUtils.hpp +++ b/lib/utils/FileUtils.hpp @@ -11,8 +11,6 @@ namespace MAT_NS_BEGIN { - static constexpr size_t MAX_FILE_CONTENTS_SIZE = 4096; - size_t FileGetSize(const char* filename); int FileDelete(const char* filename); std::FILE* FileOpen(const char* filename, const char *mode); diff --git a/lib/utils/Utils.cpp b/lib/utils/Utils.cpp index d77b0b6bc..a1cf48ee7 100644 --- a/lib/utils/Utils.cpp +++ b/lib/utils/Utils.cpp @@ -40,52 +40,6 @@ namespace MAT_NS_BEGIN { namespace MAT_NS_BEGIN { - std::string boundedSystemInfo(const char* value) - { - if (value == nullptr) - { - return {}; - } - size_t length = 0; - while (length <= MAX_SYSTEM_INFO_VALUE_SIZE && value[length] != '\0') - { - ++length; - } - if (length > MAX_SYSTEM_INFO_VALUE_SIZE) - { - LOG_WARN("System information value exceeds %zu bytes; truncating", MAX_SYSTEM_INFO_VALUE_SIZE); - length = MAX_SYSTEM_INFO_VALUE_SIZE; - // Do not split a UTF-8 sequence at the truncation boundary. - while (length > 0 && (static_cast(value[length]) & 0xc0) == 0x80) - { - --length; - } - } - return std::string(value, length); - } - -#ifdef _WIN32 - std::string boundedSystemInfo(const wchar_t* value) - { - if (value == nullptr) - { - return {}; - } - size_t length = 0; - while (length <= MAX_SYSTEM_INFO_VALUE_SIZE && value[length] != L'\0') - ++length; - if (length > MAX_SYSTEM_INFO_VALUE_SIZE) - { - LOG_WARN("System information value exceeds %zu UTF-16 units; truncating", MAX_SYSTEM_INFO_VALUE_SIZE); - length = MAX_SYSTEM_INFO_VALUE_SIZE; - if (value[length - 1] >= 0xd800 && value[length - 1] <= 0xdbff) - --length; - } - const std::string utf8 = to_utf8_string(std::wstring(value, length)); - return boundedSystemInfo(utf8.c_str()); - } -#endif - void sleep(unsigned delayMs) { PAL::sleep(delayMs); diff --git a/lib/utils/Utils.hpp b/lib/utils/Utils.hpp index 245ef74e1..c87c56367 100644 --- a/lib/utils/Utils.hpp +++ b/lib/utils/Utils.hpp @@ -55,16 +55,6 @@ namespace std namespace MAT_NS_BEGIN { - static constexpr size_t MAX_SYSTEM_INFO_VALUE_SIZE = 4096; - static constexpr size_t MAX_SYSTEM_INFO_SOURCE_SIZE = 64 * 1024; - static constexpr size_t MAX_SYSTEM_INFO_BLOB_SIZE = 1024 * 1024; - - // Bounds SDK-collected UTF-8 metadata, not caller-supplied event properties. - std::string boundedSystemInfo(const char* value); -#ifdef _WIN32 - std::string boundedSystemInfo(const wchar_t* value); -#endif - const char* getMATSDKLogComponent(); typedef std::chrono::milliseconds ms; diff --git a/tests/unittests/LogSessionDataTests.cpp b/tests/unittests/LogSessionDataTests.cpp index d48c4d40b..044e0bcc8 100644 --- a/tests/unittests/LogSessionDataTests.cpp +++ b/tests/unittests/LogSessionDataTests.cpp @@ -76,19 +76,14 @@ TEST(LogSessionDataTests, parse_ValidCrlfInput_ReturnsTrue) EXPECT_EQ(sessionSDKUid, "bar"); } -TEST(LogSessionDataTests, parse_TrailingData_ReturnsFalse) +TEST(LogSessionDataTests, parse_LargeIdentifier_PreservesExistingBehavior) { TestLogSessionDataProvider provider(PathToTestSesFile); - EXPECT_FALSE(provider.parse("1234567890\nbar\nextra", sessionFirstTimeLaunch, sessionSDKUid)); - EXPECT_FALSE(provider.parse("1234567890\r\nbar\r\n\x1a" "hidden", + const std::string identifier(16 * 1024, 'x'); + ASSERT_TRUE(provider.parse("1234567890\n" + identifier + "\n", sessionFirstTimeLaunch, sessionSDKUid)); -} - -TEST(LogSessionDataTests, parse_OversizedInput_ReturnsFalse) -{ - TestLogSessionDataProvider provider(PathToTestSesFile); - const std::string content = "1234567890\n" + std::string(MAX_FILE_CONTENTS_SIZE, 'x') + "\n"; - ASSERT_FALSE(provider.parse(content, sessionFirstTimeLaunch, sessionSDKUid)); + EXPECT_EQ(sessionFirstTimeLaunch, uint64_t{1234567890}); + EXPECT_EQ(sessionSDKUid, identifier); } TEST(LogSessionDataTests, getLogSessionData_ValidInput_SessionDataPersists) @@ -145,15 +140,13 @@ TEST_F(LogSessionFileTests, PreservesExistingCrlfSession) EXPECT_EQ(provider.GetLogSessionData()->getSessionSDKUid(), "original-id"); } -TEST_F(LogSessionFileTests, RegeneratesOversizedSessionWithControlZTail) +TEST_F(LogSessionFileTests, PreservesLargeSessionIdentifier) { - write("1234567890\r\noriginal-id\r\n" + std::string(1, '\x1a') + - std::string(MAX_FILE_CONTENTS_SIZE, 'x')); + const std::string identifier(16 * 1024, 'x'); + write("1234567890\n" + identifier + "\n"); TestLogSessionDataProvider provider(cachePath); provider.CreateLogSessionData(); ASSERT_NE(provider.GetLogSessionData(), nullptr); - EXPECT_NE(provider.GetLogSessionData()->getSessionFirstTime(), uint64_t{1234567890}); - EXPECT_NE(provider.GetLogSessionData()->getSessionSDKUid(), "original-id"); - EXPECT_FALSE(provider.GetLogSessionData()->getSessionSDKUid().empty()); - EXPECT_LE(FileGetContents(sessionPath.c_str()).size(), MAX_FILE_CONTENTS_SIZE); + EXPECT_EQ(provider.GetLogSessionData()->getSessionFirstTime(), uint64_t{1234567890}); + EXPECT_EQ(provider.GetLogSessionData()->getSessionSDKUid(), identifier); } diff --git a/tests/unittests/bounded-input-tests.cpp b/tests/unittests/bounded-input-tests.cpp index 852cb09e2..59f3b3eaf 100644 --- a/tests/unittests/bounded-input-tests.cpp +++ b/tests/unittests/bounded-input-tests.cpp @@ -42,66 +42,11 @@ class BoundedInputTests : public testing::Test } }; -TEST_F(BoundedInputTests, BoundsMetadataAtExactByteLimit) +TEST_F(BoundedInputTests, PreservesLargeTextFiles) { - EXPECT_EQ(boundedSystemInfo(static_cast(nullptr)), ""); - EXPECT_EQ(boundedSystemInfo("ordinary"), "ordinary"); - const std::string exact(MAX_SYSTEM_INFO_VALUE_SIZE, 'x'); - EXPECT_EQ(boundedSystemInfo(exact.c_str()), exact); - EXPECT_EQ(boundedSystemInfo((exact + "extra").c_str()), exact); -} - -TEST_F(BoundedInputTests, DoesNotSplitUtf8Metadata) -{ - const std::string prefix(MAX_SYSTEM_INFO_VALUE_SIZE - 1, 'x'); - EXPECT_EQ(boundedSystemInfo((prefix + "\xc3\xa9").c_str()), prefix); - const std::string exact(MAX_SYSTEM_INFO_VALUE_SIZE - 2, 'x'); - EXPECT_EQ(boundedSystemInfo((exact + "\xc3\xa9" + "z").c_str()), exact + "\xc3\xa9"); -} - -#ifdef _WIN32 -TEST_F(BoundedInputTests, BoundsUtf16BeforeConversion) -{ - const std::wstring input(MAX_SYSTEM_INFO_VALUE_SIZE + 1, L'x'); - EXPECT_EQ(boundedSystemInfo(input.c_str()), std::string(MAX_SYSTEM_INFO_VALUE_SIZE, 'x')); - const std::wstring unicode(MAX_SYSTEM_INFO_VALUE_SIZE, L'\u00e9'); - EXPECT_EQ(boundedSystemInfo(unicode.c_str()).size(), MAX_SYSTEM_INFO_VALUE_SIZE); -} -#endif - -TEST_F(BoundedInputTests, RejectsOversizedFilesInsteadOfParsingAPrefix) -{ - const std::string exact(MAX_FILE_CONTENTS_SIZE, 'x'); - write(exact); - EXPECT_EQ(FileGetContents(path.c_str()), exact); - write(exact + "x"); - EXPECT_TRUE(FileGetContents(path.c_str()).empty()); - write(std::string(100 * 1024, 'x')); - EXPECT_TRUE(FileGetContents(path.c_str()).empty()); -} - -TEST_F(BoundedInputTests, CountsPhysicalCrlfBytesAtTheFileLimit) -{ - std::string exact; - for (size_t i = 0; i < MAX_FILE_CONTENTS_SIZE / 2; ++i) - { - exact += "\r\n"; - } - write(exact); - EXPECT_EQ(FileGetContents(path.c_str()), exact); - write(exact + "x"); - EXPECT_TRUE(FileGetContents(path.c_str()).empty()); -} - -TEST_F(BoundedInputTests, DoesNotTreatControlZAsEndOfFile) -{ - const std::string prefix = "1234567890\r\noriginal-id\r\n"; - const std::string exact = prefix + '\x1a' + - std::string(MAX_FILE_CONTENTS_SIZE - prefix.size() - 1, 'x'); - write(exact); - EXPECT_EQ(FileGetContents(path.c_str()), exact); - write(exact + "x"); - EXPECT_TRUE(FileGetContents(path.c_str()).empty()); + const std::string content(100 * 1024, 'x'); + write(content); + EXPECT_EQ(FileGetContents(path.c_str()), content); } TEST_F(BoundedInputTests, RejectsImpossibleContainerCountsBeforeAllocation) @@ -113,18 +58,33 @@ TEST_F(BoundedInputTests, RejectsImpossibleContainerCountsBeforeAllocation) EXPECT_FALSE(reader.ReadContainerBegin(size, type)); } -TEST_F(BoundedInputTests, EnforcesContainerElementLimit) +TEST_F(BoundedInputTests, AcceptsLargeContainersWhenTheirElementsFitTheInput) { - std::vector input = {bond_lite::BT_BOOL, 0x80, 0x80, 0x04}; - input.resize(input.size() + bond_lite::CompactBinaryProtocolReader::MAX_CONTAINER_ELEMENTS, 0); + constexpr uint32_t count = 65537; + std::vector input = {bond_lite::BT_BOOL, 0x81, 0x80, 0x04}; + input.resize(input.size() + count, 0); bond_lite::CompactBinaryProtocolReader reader(input); uint32_t size; uint8_t type; EXPECT_TRUE(reader.ReadContainerBegin(size, type)); - input[1] = 0x81; - input.push_back(0); - bond_lite::CompactBinaryProtocolReader oversizedReader(input); - EXPECT_FALSE(oversizedReader.ReadContainerBegin(size, type)); + EXPECT_EQ(size, count); + + std::vector map = {bond_lite::BT_BOOL, bond_lite::BT_BOOL, 0x81, 0x80, 0x04}; + map.resize(map.size() + 2 * count, 0); + bond_lite::CompactBinaryProtocolReader mapReader(map); + uint8_t valueType; + EXPECT_TRUE(mapReader.ReadMapContainerBegin(size, type, valueType)); + EXPECT_EQ(size, count); +} + +TEST_F(BoundedInputTests, RejectsMapCountsThatCannotFitTheInput) +{ + const std::vector input = {bond_lite::BT_BOOL, bond_lite::BT_BOOL, 2, 0, 0, 0}; + bond_lite::CompactBinaryProtocolReader reader(input); + uint32_t size; + uint8_t keyType; + uint8_t valueType; + EXPECT_FALSE(reader.ReadMapContainerBegin(size, keyType, valueType)); } #if !defined(_WIN32) && !defined(ANDROID) @@ -137,14 +97,37 @@ TEST_F(BoundedInputTests, ReadsOnlyArgvZeroFromLongNulSeparatedCommandLine) EXPECT_EQ(sources.get("appId"), executable); } -TEST_F(BoundedInputTests, BoundsLongArgvZeroAndRawMetadata) +TEST_F(BoundedInputTests, BoundsOnlyLongArgvZeroAndPreservesRawMetadata) { - write(std::string(100 * 1024, 'x') + '\0' + "argument"); + const std::string content = std::string(100 * 1024, 'x') + '\0' + "argument"; + write(content); sysinfo_sources sources; sources.add("appId", {path.c_str(), sysinfo_selector::first_null}); sources.add("raw", {path.c_str(), sysinfo_selector::raw}); - EXPECT_EQ(sources.get("appId"), std::string(MAX_SYSTEM_INFO_VALUE_SIZE, 'x')); - EXPECT_EQ(sources.get("raw").size(), MAX_SYSTEM_INFO_VALUE_SIZE); + EXPECT_EQ(sources.get("appId"), std::string(MAX_COMMAND_LINE_SIZE, 'x')); + EXPECT_EQ(sources.get("raw"), content); +} + +TEST_F(BoundedInputTests, CommandLineLimitPreservesUtf8Boundaries) +{ + const std::string characters[] = {"\xc3\xa9", "\xe2\x82\xac", "\xf0\x9f\x98\x80"}; + for (const auto& character : characters) + { + for (size_t remaining = 1; remaining < character.size(); ++remaining) + { + const std::string prefix(MAX_COMMAND_LINE_SIZE - remaining, 'x'); + write(prefix + character + '\0'); + sysinfo_sources truncated; + truncated.add("appId", {path.c_str(), sysinfo_selector::first_null}); + EXPECT_EQ(truncated.get("appId"), prefix); + } + + const std::string exact = std::string(MAX_COMMAND_LINE_SIZE - character.size(), 'x') + character; + write(exact + '\0' + "ignored argument"); + sysinfo_sources accepted; + accepted.add("appId", {path.c_str(), sysinfo_selector::first_null}); + EXPECT_EQ(accepted.get("appId"), exact); + } } TEST_F(BoundedInputTests, ReadsOsReleaseKeysWithoutSuffixMatchesOrRegex) @@ -161,12 +144,13 @@ TEST_F(BoundedInputTests, ReadsOsReleaseKeysWithoutSuffixMatchesOrRegex) EXPECT_EQ(sources.get("osRel"), "24.04 (LTS)"); } -TEST_F(BoundedInputTests, BoundsOsReleaseValuesAndReadsUnterminatedFinalLine) +TEST_F(BoundedInputTests, PreservesLargeOsReleaseValuesAndReadsUnterminatedFinalLine) { - write("ID=linux\nVERSION=" + std::string(100 * 1024, 'x')); + const std::string version(100 * 1024, 'x'); + write("IGNORED=" + std::string(100 * 1024, 'y') + "\nID=linux\nVERSION=" + version); sysinfo_sources sources; sources.add("version", {path.c_str(), sysinfo_selector::key_value, "VERSION"}); - EXPECT_EQ(sources.get("version").size(), MAX_SYSTEM_INFO_VALUE_SIZE); + EXPECT_EQ(sources.get("version"), version); write("ID=linux"); sysinfo_sources finalLine; finalLine.add("id", {path.c_str(), sysinfo_selector::key_value, "ID"}); @@ -209,7 +193,7 @@ TEST_F(BoundedInputTests, ActualCommandLineOnSmallStack) ASSERT_EQ(pthread_join(thread, nullptr), 0); EXPECT_FALSE(result.empty()); EXPECT_EQ(result.find('\0'), std::string::npos); - EXPECT_LE(result.size(), MAX_SYSTEM_INFO_VALUE_SIZE); + EXPECT_LE(result.size(), MAX_COMMAND_LINE_SIZE); } TEST_F(BoundedInputTests, InitializesTelemetryWithA100KbCommandLine) diff --git a/tests/unittests/device-id-tests.cpp b/tests/unittests/device-id-tests.cpp index ad368125c..b1b8cede2 100644 --- a/tests/unittests/device-id-tests.cpp +++ b/tests/unittests/device-id-tests.cpp @@ -6,7 +6,6 @@ #include "common/Common.hpp" #include "api/ContextFieldsProvider.hpp" -#include "utils/Utils.hpp" #if !defined(_WIN32) && !defined(ANDROID) #include "pal/posix/sysinfo_sources_impl.hpp" @@ -46,7 +45,7 @@ TEST(DeviceIdTests, SemanticRegistrationPreservesCallerIdWhenCollectionIsDisable #ifdef MATSDK_DISABLE_DEVICE_ID const std::string expected = callerId; #else - const std::string expected = boundedSystemInfo(PAL::GetDeviceInformation()->GetDeviceId().c_str()); + const std::string expected = PAL::GetDeviceInformation()->GetDeviceId(); #endif EXPECT_EQ(std::string(fields.at(COMMONFIELDS_DEVICE_ID).as_string), expected); #ifdef MATSDK_DISABLE_DEVICE_ID @@ -70,7 +69,7 @@ TEST(DeviceIdTests, RegistrationRetainsOtherMetadataWithoutInventingAnId) ASSERT_EQ(fields.count(COMMONFIELDS_OS_NAME), 1u); ASSERT_EQ(fields.count(COMMONFIELDS_APP_ID), 1u); EXPECT_EQ(std::string(fields.at(COMMONFIELDS_OS_NAME).as_string), - boundedSystemInfo(PAL::GetSystemInformation()->GetOsName().c_str())); + PAL::GetSystemInformation()->GetOsName()); EXPECT_EQ(std::string(fields.at(COMMONFIELDS_APP_ID).as_string), - boundedSystemInfo(PAL::GetSystemInformation()->GetAppId().c_str())); + PAL::GetSystemInformation()->GetAppId()); }