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/.github/workflows/build-posix-latest.yml b/.github/workflows/build-posix-latest.yml index 95c387fd0..fa75b6733 100644 --- a/.github/workflows/build-posix-latest.yml +++ b/.github/workflows/build-posix-latest.yml @@ -49,6 +49,37 @@ 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 }} + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" + 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: 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 + 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/CsProtocol-decoding.md b/docs/CsProtocol-decoding.md index c02901780..1737795ad 100644 --- a/docs/CsProtocol-decoding.md +++ b/docs/CsProtocol-decoding.md @@ -28,3 +28,23 @@ 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 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 +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..ba4cae4fd 100644 --- a/docs/PAL.md +++ b/docs/PAL.md @@ -59,6 +59,61 @@ ## 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. 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. + +## Targeted input safeguards + +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 | +| 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. +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 +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. + +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/docs/building-with-vcpkg.md b/docs/building-with-vcpkg.md index 36feeea17..5e37272cb 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/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/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..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 @@ -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,56 @@ 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 remaining = MAX_HTTP_RESPONSE_HEADERS_SIZE; + for (Map.Entry> entry : headers.entrySet()) { + if (entry.getKey() != null) { + for (String value : entry.getValue()) { + if (remaining < 4) { + throw new IOException("HTTP response headers exceed buffered size limit"); + } + remaining = remainingHeaderBytes(entry.getKey(), remaining - 4); + remaining = remainingHeaderBytes(value, remaining); + result.add(entry.getKey()); + result.add(value); + } + } + } + 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, @@ -111,8 +164,6 @@ class Request implements HttpClientRequest { } public void run() { - final boolean logExceptions = false; - String[] headerArray = {}; byte[] body = {}; int response = 0; @@ -122,49 +173,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(); } @@ -220,7 +241,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(); @@ -310,7 +331,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 @@ -372,6 +396,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 523bcc9f3..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 @@ -13,11 +13,80 @@ 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 disabledDeviceIdDoesNotAccessAndroidContext() { + Assert.assertEquals("", HttpClient.calculateID(null, false)); + } + + @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 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); 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/bond/CompactBinaryProtocolReader.hpp b/lib/bond/CompactBinaryProtocolReader.hpp index 708aba421..0dc336bae 100644 --- a/lib/bond/CompactBinaryProtocolReader.hpp +++ b/lib/bond/CompactBinaryProtocolReader.hpp @@ -186,6 +186,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 +218,16 @@ class CompactBinaryProtocolReader { } elementType = (raw & 31); - return ReadUInt32(size); + return ReadUInt32(size) && + 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 <= (m_input.size() - m_ofs) / 2; } bool ReadContainerEnd() @@ -277,4 +283,3 @@ class CompactBinaryProtocolReader { }; } // namespace bond_lite - diff --git a/lib/decoder/PayloadDecoder.cpp b/lib/decoder/PayloadDecoder.cpp index 6d86a7057..5c8ad7adf 100644 --- a/lib/decoder/PayloadDecoder.cpp +++ b/lib/decoder/PayloadDecoder.cpp @@ -73,12 +73,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++; } @@ -86,7 +85,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; @@ -97,7 +96,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 +104,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 +540,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 edd3497a9..eac40ad13 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 @@ -506,6 +503,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))) { @@ -513,7 +512,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))) @@ -560,17 +559,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) @@ -701,10 +689,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; } @@ -1044,7 +1036,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 { @@ -1106,6 +1098,16 @@ class CurlHttpOperation { * @return */ static size_t WriteVectorCallback(char* ptr, size_t size, size_t nmemb, void* userp) noexcept + { + 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) { @@ -1114,11 +1116,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 a96cdcebd..7dc17457f 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..f4885eff3 100644 --- a/lib/offline/LogSessionDataProvider.cpp +++ b/lib/offline/LogSessionDataProvider.cpp @@ -213,6 +213,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/pal/PAL.cpp b/lib/pal/PAL.cpp index c9d750232..59873c282 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(m_DeviceInformation->GetDeviceId()); + const auto& deviceId = m_DeviceInformation->GetDeviceId(); +#ifdef MATSDK_DISABLE_DEVICE_ID + if (!deviceId.empty()) +#endif + context->SetDeviceId(deviceId); context->SetDeviceModel(m_DeviceInformation->GetModel()); context->SetDeviceMake(m_DeviceInformation->GetManufacturer()); } diff --git a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp index 3c8fe6baf..79a815fad 100644 --- a/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopDeviceInformationImpl.cpp @@ -13,13 +13,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)) @@ -34,11 +36,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. */ @@ -76,6 +81,7 @@ namespace PAL_NS_BEGIN { } return devId; } +#endif // Helper functions. PowerSource GetCurrentPowerSource() @@ -110,7 +116,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/desktop/WindowsDesktopSystemInformationImpl.cpp b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp index 8bc6b1147..17eea75df 100644 --- a/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp +++ b/lib/pal/desktop/WindowsDesktopSystemInformationImpl.cpp @@ -120,7 +120,6 @@ namespace PAL_NS_BEGIN { { return{}; } - buffer.resize(dwVersionInfoSize); if (GetFileVersionInfo(applicationFullPath.data(), 0, dwVersionInfoSize, &buffer[0]) == 0) @@ -190,10 +189,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 +248,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 +260,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 +326,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - 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 fc132629d..0f4036599 100644 --- a/lib/pal/posix/DeviceInformationImpl_Android.cpp +++ b/lib/pal/posix/DeviceInformationImpl_Android.cpp @@ -44,7 +44,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; @@ -170,6 +174,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"); @@ -181,25 +194,19 @@ 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 += jStr; pEnv->ReleaseStringUTFChars(androidIdJstr, jStr); + AndroidDeviceInformationConnector::setDeviceId(std::move(deviceId)); +#endif jStr = pEnv->GetStringUTFChars(manufacturerJstr, &isCopy); std::string manufacturer = jStr; @@ -209,12 +216,22 @@ namespace PAL_NS_BEGIN { std::string model = 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 28f1e3552..14ddcf0aa 100644 --- a/lib/pal/posix/sysinfo_sources.cpp +++ b/lib/pal/posix/sysinfo_sources.cpp @@ -10,6 +10,7 @@ */ #include "sysinfo_sources_impl.hpp" +#include "pal/PAL.hpp" #include #include @@ -25,8 +26,6 @@ #include #include -#include - #include #include @@ -55,7 +54,7 @@ #include "TargetConditionals.h" #include "sysinfo_utils_apple.hpp" -#ifdef TARGET_MAC_OS +#if defined(TARGET_MAC_OS) && !defined(MATSDK_DISABLE_DEVICE_ID) #include @@ -73,12 +72,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) + { + LOG_WARN("Executable path length is empty"); + 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 +100,41 @@ 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; + auto mode = std::ios::in; + if (selector == sysinfo_selector::first_null) + mode |= std::ios::binary; + std::ifstream input(filename, mode); + std::string result; + char ch; + while (input.get(ch)) + { + if ((selector == sysinfo_selector::first_null && ch == '\0') || + (selector == sysinfo_selector::first_line && ch == '\n')) + { + return result; + } + 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; + } + result.push_back(ch); + } + if (input.bad()) + { + LOG_WARN("Unable to read system information source"); + return {}; + } + return result; } /** @@ -103,31 +142,30 @@ inline std::string ReadFile(const char *filename) * @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; 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())) + 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()); + result.append(buffer.data(), count); + } + if (ferror(pipe.get())) + { + LOG_WARN("Unable to read system information command output"); + return {}; } - // Remove EOL. In all use-cases below we don't need it. if (!result.empty() && result[result.length()-1]=='\n') { @@ -136,44 +174,54 @@ static std::string Exec(const char* cmd) return result; } -#if defined(__clang__) -#pragma clang diagnostic pop -#elif defined(__GNUC__) -#pragma GCC diagnostic pop #endif /** - * Read node value, preprocess it using regexp and store result 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 */ 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] = std::move(contents); + 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] = std::move(value); + return true; + } + } + return false; } /** @@ -218,24 +266,13 @@ 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); - } - } +#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"}); + add("osBuild", {"/proc/version", sysinfo_selector::first_line}); time_t t = time(NULL); @@ -268,9 +305,11 @@ 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", "*"}); +#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 #if defined(__APPLE__) @@ -316,11 +355,12 @@ 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 +#ifndef MATSDK_DISABLE_DEVICE_ID if (!get("devId").compare("")) { #ifdef __APPLE__ @@ -348,5 +388,6 @@ sysinfo_sources_impl::sysinfo_sources_impl() : sysinfo_sources() } #endif } +#endif } diff --git a/lib/pal/posix/sysinfo_sources.hpp b/lib/pal/posix/sysinfo_sources.hpp index 427ac31f9..035f027c4 100644 --- a/lib/pal/posix/sysinfo_sources.hpp +++ b/lib/pal/posix/sysinfo_sources.hpp @@ -7,14 +7,31 @@ #include #include +#include + +static constexpr std::size_t MAX_COMMAND_LINE_SIZE = 4096; /** * 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 +46,7 @@ class sysinfo_sources : public std::multimap { std::map cache; /** - * Read node value, preprocess it using regexp and store result 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 @@ -60,4 +77,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..e5fcebc30 100644 --- a/lib/pal/posix/sysinfo_utils_ios.mm +++ b/lib/pal/posix/sysinfo_utils_ios.mm @@ -73,6 +73,7 @@ std::string GetDeviceId() { +#ifndef MATSDK_DISABLE_DEVICE_ID @autoreleasepool { NSUUID *nsuuid = [[UIDevice currentDevice] identifierForVendor]; if (nsuuid) @@ -86,6 +87,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 193187fd3..60c1cfa68 100644 --- a/lib/pal/posix/sysinfo_utils_mac.mm +++ b/lib/pal/posix/sysinfo_utils_mac.mm @@ -4,9 +4,12 @@ // #include "sysinfo_utils_apple.hpp" +#include "pal/PAL.hpp" #include #include +#ifndef MATSDK_DISABLE_DEVICE_ID #include +#endif #include #import @@ -14,13 +17,19 @@ 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) + { + LOG_WARN("Device model length is empty"); + return {}; + } + deviceModelBuffer.resize(size + 1, '\0'); if (sysctlbyname(query, deviceModelBuffer.data(), &size, nullptr, 0) == 0) { deviceModel = deviceModelBuffer.data(); @@ -37,6 +46,7 @@ std::string GetDeviceId() { +#ifndef MATSDK_DISABLE_DEVICE_ID uuid_t uuidBytes; const struct timespec spec = {1, 0}; int hostUUIDResult = gethostuuid(uuidBytes, &spec); @@ -49,6 +59,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 486945d09..5ab652bd7 100644 --- a/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp +++ b/lib/pal/universal/WindowsRuntimeDeviceInformationImpl.cpp @@ -58,6 +58,7 @@ namespace PAL_NS_BEGIN { m_model = FromPlatformString(easClientDeviceInformation->SystemProductName); m_manufacturer = FromPlatformString(easClientDeviceInformation->SystemManufacturer); +#ifndef MATSDK_DISABLE_DEVICE_ID bool isNetDetectEnabled = configuration[CFG_BOOL_ENABLE_NET_DETECT]; m_device_id = DEFAULT_DEVICE_ID; @@ -86,6 +87,9 @@ 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 #else // Windows 8.1 SDK @@ -149,4 +153,3 @@ namespace PAL_NS_BEGIN { } } PAL_NS_END - diff --git a/lib/utils/FileUtils.cpp b/lib/utils/FileUtils.cpp index 506bee9b3..fa43f37aa 100644 --- a/lib/utils/FileUtils.cpp +++ b/lib/utils/FileUtils.cpp @@ -111,13 +111,23 @@ namespace MAT_NS_BEGIN { result += buff; } + if (ferror(fp)) + { + 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; + 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 } @@ -170,4 +180,3 @@ namespace MAT_NS_BEGIN } } MAT_NS_END - diff --git a/lib/utils/FileUtils.hpp b/lib/utils/FileUtils.hpp index cefce7627..620eba4be 100644 --- a/lib/utils/FileUtils.hpp +++ b/lib/utils/FileUtils.hpp @@ -22,4 +22,3 @@ namespace MAT_NS_BEGIN } MAT_NS_END #endif - diff --git a/lib/utils/Utils.hpp b/lib/utils/Utils.hpp index ce249b9ab..c87c56367 100644 --- a/lib/utils/Utils.hpp +++ b/lib/utils/Utils.hpp @@ -99,4 +99,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/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/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; } diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index bcdb0acb8..14502a66f 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -6,12 +6,14 @@ set(SRCS AnnexKTests.cpp BackoffTests_ExponentialWithJitter.cpp BondSplicerTests.cpp + bounded-input-tests.cpp ClockSkewManagerTests.cpp ContextFieldsProviderTests.cpp ControlPlaneProviderTests.cpp 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/HttpClientCurlTests.cpp b/tests/unittests/HttpClientCurlTests.cpp index 792229917..1d6236e19 100644 --- a/tests/unittests/HttpClientCurlTests.cpp +++ b/tests/unittests/HttpClientCurlTests.cpp @@ -100,6 +100,27 @@ 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 CountingHttpServer : public HttpServer { public: @@ -152,6 +173,7 @@ class HttpClientCurlHeaderTests : public ::testing::Test, protected: CountingHttpServer m_server; std::string m_url; + std::map m_responseHeaders {{"X-MAT-Test", "header-value"}}; std::mutex m_requestMutex; std::string m_requestMethod; std::string m_requestContent; @@ -177,7 +199,7 @@ class HttpClientCurlHeaderTests : public ::testing::Test, std::lock_guard lock(m_requestMutex); m_requestMethod = request.method; m_requestContent = request.content; - response.headers["X-MAT-Test"] = "header-value"; + response.headers = m_responseHeaders; response.content = "body-value"; return 200; } @@ -220,6 +242,34 @@ TEST_F(HttpClientCurlHeaderTests, CapturesResponseHeadersAndBody) EXPECT_EQ(m_server.acceptedConnections(), UsesPrereqCallback() ? 1u : 2u); } +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); +} + TEST_F(HttpClientCurlHeaderTests, SendsBinaryPostWithoutRedundantConnection) { const std::map requestHeaders; 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..044e0bcc8 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,24 @@ 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_LargeIdentifier_PreservesExistingBehavior) +{ + TestLogSessionDataProvider provider(PathToTestSesFile); + const std::string identifier(16 * 1024, 'x'); + ASSERT_TRUE(provider.parse("1234567890\n" + identifier + "\n", + sessionFirstTimeLaunch, sessionSDKUid)); + EXPECT_EQ(sessionFirstTimeLaunch, uint64_t{1234567890}); + EXPECT_EQ(sessionSDKUid, identifier); +} + TEST(LogSessionDataTests, getLogSessionData_ValidInput_SessionDataPersists) { const std::string sessionFile = @@ -88,3 +107,46 @@ 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, PreservesLargeSessionIdentifier) +{ + const std::string identifier(16 * 1024, 'x'); + write("1234567890\n" + identifier + "\n"); + TestLogSessionDataProvider provider(cachePath); + provider.CreateLogSessionData(); + ASSERT_NE(provider.GetLogSessionData(), nullptr); + EXPECT_EQ(provider.GetLogSessionData()->getSessionFirstTime(), uint64_t{1234567890}); + EXPECT_EQ(provider.GetLogSessionData()->getSessionSDKUid(), identifier); +} diff --git a/tests/unittests/PayloadDecoderTests.cpp b/tests/unittests/PayloadDecoderTests.cpp index 8e8051cee..5ec715f54 100644 --- a/tests/unittests/PayloadDecoderTests.cpp +++ b/tests/unittests/PayloadDecoderTests.cpp @@ -4,10 +4,33 @@ // #include "common/Common.hpp" #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; +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 @@ -33,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 89da8f8be..ec66e75b6 100644 --- a/tests/unittests/UnitTests.vcxproj +++ b/tests/unittests/UnitTests.vcxproj @@ -432,12 +432,14 @@ + + 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..59f3b3eaf --- /dev/null +++ b/tests/unittests/bounded-input-tests.cpp @@ -0,0 +1,215 @@ +// +// 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, PreservesLargeTextFiles) +{ + const std::string content(100 * 1024, 'x'); + write(content); + EXPECT_EQ(FileGetContents(path.c_str()), content); +} + +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, AcceptsLargeContainersWhenTheirElementsFitTheInput) +{ + 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)); + 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) +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, BoundsOnlyLongArgvZeroAndPreservesRawMetadata) +{ + 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_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) +{ + 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, PreservesLargeOsReleaseValuesAndReadsUnterminatedFinalLine) +{ + 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"), version); + 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_COMMAND_LINE_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 diff --git a/tests/unittests/device-id-tests.cpp b/tests/unittests/device-id-tests.cpp new file mode 100644 index 000000000..b1b8cede2 --- /dev/null +++ b/tests/unittests/device-id-tests.cpp @@ -0,0 +1,75 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// + +#include "common/Common.hpp" + +#include "api/ContextFieldsProvider.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 = PAL::GetDeviceInformation()->GetDeviceId(); +#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), + PAL::GetSystemInformation()->GetOsName()); + EXPECT_EQ(std::string(fields.at(COMMONFIELDS_APP_ID).as_string), + PAL::GetSystemInformation()->GetAppId()); +} 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." },