Sanitize sensitive data from URLs (span desc, span data, crumbs, client errors) (#1327)

Co-authored-by: Manoel Aranda Neto <[email protected]>
Co-authored-by: Manoel Aranda Neto <[email protected]>

Author: denrase

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Features
+
+- Sanitize sensitive data from URLs (span desc, span data, crumbs, client errors) ([#1327](https://github.com/getsentry/sentry-dart/pull/1327))
+
 ### Dependencies
 
 - Bump Cocoa SDK from v8.3.1 to v8.3.3 ([#1350](https://github.com/getsentry/sentry-dart/pull/1350), [#1355](https://github.com/getsentry/sentry-dart/pull/1355))

dart/lib/sentry.dart

@@ -39,3 +39,8 @@ export 'src/exception_stacktrace_extractor.dart';
 // Isolates
 export 'src/sentry_isolate_extension.dart';
 export 'src/sentry_isolate.dart';
+// URL
+// ignore: invalid_export_of_internal_element
+export 'src/utils/http_sanitizer.dart';
+// ignore: invalid_export_of_internal_element
+export 'src/utils/url_details.dart';

dart/lib/sentry_io.dart

@@ -1,2 +1,3 @@
+// ignore: invalid_export_of_internal_element
 export 'sentry.dart';
 export 'src/sentry_attachment/io_sentry_attachment.dart';

dart/lib/src/event_processor/enricher/web_enricher_event_processor.dart

@@ -49,10 +49,12 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {
 
     header.putIfAbsent('User-Agent', () => _window.navigator.userAgent);
 
-    return (request ?? SentryRequest()).copyWith(
-      url: request?.url ?? _window.location.toString(),
-      headers: header,
-    );
+    final url = request?.url ?? _window.location.toString();
+    return (request ?? SentryRequest(url: url))
+        .copyWith(
+          headers: header,
+        )
+        .sanitized();
   }
 
   SentryDevice _getDevice(SentryDevice? device) {

dart/lib/src/http_client/breadcrumb_client.dart

@@ -2,6 +2,8 @@ import 'package:http/http.dart';
 import '../protocol.dart';
 import '../hub.dart';
 import '../hub_adapter.dart';
+import '../utils/url_details.dart';
+import '../utils/http_sanitizer.dart';
 
 /// A [http](https://pub.dev/packages/http)-package compatible HTTP client
 /// which records requests as breadcrumbs.
@@ -75,15 +77,20 @@ class BreadcrumbClient extends BaseClient {
     } finally {
       stopwatch.stop();
 
+      final urlDetails =
+          HttpSanitizer.sanitizeUrl(request.url.toString()) ?? UrlDetails();
+
       var breadcrumb = Breadcrumb.http(
         level: requestHadException ? SentryLevel.error : SentryLevel.info,
-        url: request.url,
+        url: Uri.parse(urlDetails.urlOrFallback),
         method: request.method,
         statusCode: statusCode,
         reason: reason,
         requestDuration: stopwatch.elapsed,
         requestBodySize: request.contentLength,
         responseBodySize: responseBodySize,
+        httpQuery: urlDetails.query,
+        httpFragment: urlDetails.fragment,
       );
 
       await _hub.addBreadcrumb(breadcrumb);

dart/lib/src/http_client/tracing_client.dart

@@ -4,6 +4,7 @@ import '../hub_adapter.dart';
 import '../protocol.dart';
 import '../tracing.dart';
 import '../utils/tracing_utils.dart';
+import '../utils/http_sanitizer.dart';
 
 /// A [http](https://pub.dev/packages/http)-package compatible HTTP client
 /// which adds support to Sentry Performance feature.
@@ -19,17 +20,28 @@ class TracingClient extends BaseClient {
   @override
   Future<StreamedResponse> send(BaseRequest request) async {
     // see https://develop.sentry.dev/sdk/performance/#header-sentry-trace
+
+    final urlDetails = HttpSanitizer.sanitizeUrl(request.url.toString());
+
+    var description = request.method;
+    if (urlDetails != null) {
+      description += ' ${urlDetails.urlOrFallback}';
+    }
+
     final currentSpan = _hub.getSpan();
     var span = currentSpan?.startChild(
       'http.client',
-      description: '${request.method} ${request.url}',
+      description: description,
     );
 
-    // if the span is NoOp, we dont want to attach headers
+    // if the span is NoOp, we don't want to attach headers
     if (span is NoOpSentrySpan) {
       span = null;
     }
 
+    span?.setData('method', request.method);
+    urlDetails?.applyToSpan(span);
+
     StreamedResponse? response;
     try {
       if (span != null) {

dart/lib/src/protocol/breadcrumb.dart

@@ -3,7 +3,7 @@ import 'package:meta/meta.dart';
 import '../utils.dart';
 import '../protocol.dart';
 
-/// Structed data to describe more information pior to the event captured.
+/// Structured data to describe more information prior to the event captured.
 /// See `Sentry.captureEvent()`.
 ///
 /// The outgoing JSON representation is:
@@ -47,6 +47,8 @@ class Breadcrumb {
 
     // Size of the response body in bytes
     int? responseBodySize,
+    String? httpQuery,
+    String? httpFragment,
   }) {
     return Breadcrumb(
       type: 'http',
@@ -61,6 +63,8 @@ class Breadcrumb {
         if (requestDuration != null) 'duration': requestDuration.toString(),
         if (requestBodySize != null) 'request_body_size': requestBodySize,
         if (responseBodySize != null) 'response_body_size': responseBodySize,
+        if (httpQuery != null) 'http.query': httpQuery,
+        if (httpFragment != null) 'http.fragment': httpFragment,
       },
     );
   }

dart/lib/src/protocol/sentry_request.dart

@@ -1,6 +1,7 @@
 import 'package:meta/meta.dart';
 
 import '../utils/iterable_extension.dart';
+import '../utils/http_sanitizer.dart';
 
 /// The Request interface contains information on a HTTP request related to the event.
 /// In client SDKs, this can be an outgoing request, or the request that rendered the current web page.
@@ -92,31 +93,18 @@ class SentryRequest {
     @Deprecated('Will be removed in v8. Use [data] instead')
         Map<String, String>? other,
   }) {
-    // As far as I can tell there's no way to get the uri without the query part
-    // so we replace it with an empty string.
-    final urlWithoutQuery = uri
-        .replace(query: '', fragment: '')
-        .toString()
-        .replaceAll('?', '')
-        .replaceAll('#', '');
-
-    // Future proof, Dio does not support it yet and even if passing in the path,
-    // the parsing of the uri returns empty.
-    final query = uri.query.isEmpty ? null : uri.query;
-    final fragment = uri.fragment.isEmpty ? null : uri.fragment;
-
     return SentryRequest(
-      url: urlWithoutQuery,
-      fragment: fragment,
-      queryString: query,
+      url: uri.toString(),
       method: method,
       cookies: cookies,
       data: data,
       headers: headers,
       env: env,
+      queryString: uri.query,
+      fragment: uri.fragment,
       // ignore: deprecated_member_use_from_same_package
       other: other,
-    );
+    ).sanitized();
   }
 
   /// Deserializes a [SentryRequest] from JSON [Map].
@@ -162,12 +150,13 @@ class SentryRequest {
     Map<String, String>? env,
     @Deprecated('Will be removed in v8. Use [data] instead')
         Map<String, String>? other,
+    bool removeCookies = false,
   }) =>
       SentryRequest(
         url: url ?? this.url,
         method: method ?? this.method,
         queryString: queryString ?? this.queryString,
-        cookies: cookies ?? this.cookies,
+        cookies: removeCookies ? null : cookies ?? this.cookies,
         data: data ?? _data,
         headers: headers ?? _headers,
         env: env ?? _env,

dart/lib/src/utils/http_sanitizer.dart

@@ -0,0 +1,105 @@
+import 'package:meta/meta.dart';
+
+import '../protocol.dart';
+import 'url_details.dart';
+
+@internal
+class HttpSanitizer {
+  static final RegExp _authRegExp = RegExp("(.+://)(.*@)(.*)");
+  static final List<String> _securityHeaders = [
+    "X-FORWARDED-FOR",
+    "AUTHORIZATION",
+    "COOKIE",
+    "SET-COOKIE",
+    "X-API-KEY",
+    "X-REAL-IP",
+    "REMOTE-ADDR",
+    "FORWARDED",
+    "PROXY-AUTHORIZATION",
+    "X-CSRF-TOKEN",
+    "X-CSRFTOKEN",
+    "X-XSRF-TOKEN"
+  ];
+
+  /// Parse and sanitize url data for sentry.io
+  static UrlDetails? sanitizeUrl(String? url) {
+    if (url == null) {
+      return null;
+    }
+
+    final queryIndex = url.indexOf('?');
+    final fragmentIndex = url.indexOf('#');
+
+    if (queryIndex > -1 && fragmentIndex > -1 && fragmentIndex < queryIndex) {
+      // url considered malformed because of fragment position
+      return UrlDetails();
+    } else {
+      try {
+        final uri = Uri.parse(url);
+        final urlWithAuthRemoved = _urlWithAuthRemoved(uri._url());
+        return UrlDetails(
+            url: urlWithAuthRemoved.isEmpty ? null : urlWithAuthRemoved,
+            query: uri.query.isEmpty ? null : uri.query,
+            fragment: uri.fragment.isEmpty ? null : uri.fragment);
+      } catch (_) {
+        return null;
+      }
+    }
+  }
+
+  static Map<String, String>? sanitizedHeaders(Map<String, String>? headers) {
+    if (headers == null) {
+      return null;
+    }
+    final sanitizedHeaders = <String, String>{};
+    headers.forEach((key, value) {
+      if (!_securityHeaders.contains(key.toUpperCase())) {
+        sanitizedHeaders[key] = value;
+      }
+    });
+    return sanitizedHeaders;
+  }
+
+  static String _urlWithAuthRemoved(String url) {
+    final userInfoMatch = _authRegExp.firstMatch(url);
+    if (userInfoMatch != null && userInfoMatch.groupCount == 3) {
+      final userInfoString = userInfoMatch.group(2) ?? '';
+      final replacementString = userInfoString.contains(":")
+          ? "[Filtered]:[Filtered]@"
+          : "[Filtered]@";
+      return '${userInfoMatch.group(1) ?? ''}$replacementString${userInfoMatch.group(3) ?? ''}';
+    } else {
+      return url;
+    }
+  }
+}
+
+extension UriPath on Uri {
+  String _url() {
+    var buffer = '';
+    if (scheme.isNotEmpty) {
+      buffer += '$scheme://';
+    }
+    if (userInfo.isNotEmpty) {
+      buffer += '$userInfo@';
+    }
+    buffer += host;
+    if (path.isNotEmpty) {
+      buffer += path;
+    }
+    return buffer;
+  }
+}
+
+extension SanitizedSentryRequest on SentryRequest {
+  SentryRequest sanitized() {
+    final urlDetails = HttpSanitizer.sanitizeUrl(url) ?? UrlDetails();
+    return copyWith(
+      url: urlDetails.urlOrFallback,
+      queryString: urlDetails.query,
+      fragment: urlDetails.fragment,
+      headers: HttpSanitizer.sanitizedHeaders(headers),
+      removeCookies: true,
+    );
+  }
+}

dart/lib/src/utils/url_details.dart

@@ -0,0 +1,29 @@
+import 'package:meta/meta.dart';
+import '../../sentry.dart';
+
+/// Sanitized url data for sentry.io
+@internal
+class UrlDetails {
+  UrlDetails({this.url, this.query, this.fragment});
+
+  final String? url;
+  final String? query;
+  final String? fragment;
+
+  late final urlOrFallback = url ?? 'unknown';
+
+  void applyToSpan(ISentrySpan? span) {
+    if (span == null) {
+      return;
+    }
+    if (url != null) {
+      span.setData('url', url);
+    }
+    if (query != null) {
+      span.setData("http.query", query);
+    }
+    if (fragment != null) {
+      span.setData("http.fragment", fragment);
+    }
+  }
+}

dart/test/event_processor/enricher/web_enricher_test.dart

@@ -57,6 +57,23 @@ void main() {
       expect(event.request?.url, 'foo.bar');
     });
 
+    test('does not add auth headers to request', () async {
+      var event = SentryEvent(
+        request: SentryRequest(
+          url: 'foo.bar',
+          headers: {
+            'Authorization': 'foo',
+            'authorization': 'bar',
+          },
+        ),
+      );
+      var enricher = fixture.getSut();
+      event = await enricher.apply(event);
+
+      expect(event.request?.headers['Authorization'], isNull);
+      expect(event.request?.headers['authorization'], isNull);
+    });
+
     test('user-agent is not overridden if already present', () async {
       var event = SentryEvent(
         request: SentryRequest(