feat(session-replay): upload replay envelope from the native crash daemon#1809
Draft
tustanivsky wants to merge 7 commits into
Draft
feat(session-replay): upload replay envelope from the native crash daemon#1809tustanivsky wants to merge 7 commits into
tustanivsky wants to merge 7 commits into
Conversation
|
154ce2f to
b437853
Compare
Comment on lines
+313
to
+316
| sentry_value_get_by_key(meta, "videoFilename")); | ||
| sentry_path_t *mp4_path = (video_filename && video_filename[0]) | ||
| ? sentry__path_join_str(dir, video_filename) | ||
| : NULL; |
There was a problem hiding this comment.
Missing path containment for videoFilename from replay JSON sidecar allows reading files outside the replays directory
The videoFilename read from an embedder-written .json sidecar in <database>/replays/ is passed to sentry__path_join_str with no .. or absolute-path containment check, so a crafted sidecar can make the daemon read an arbitrary file and embed it in the uploaded replay_video envelope. Normalize and confirm the resolved path stays inside the replays directory before reading it.
Evidence
video_filenameis taken verbatim frommeta["videoFilename"](sentry_session_replay.c:312-313), sourced from a JSON file on disk in<database>/replays/.- It is passed directly to
sentry__path_join_str(dir, video_filename)(line 315) with no normalization or prefix check. sentry__path_join_str(sentry_path_unix.c:279) returns anyotherstarting with/as an absolute path and never strips..components, so traversal/absolute escape is possible.build_replay_envelopethen reads the full file viasentry__path_read_to_bufferand embeds it as thereplay_videoblob dispatched through the transport.- Impact is limited: exploitation requires an attacker who can already write into
<database>/replays/, and the file is uploaded to the developer's own configured Sentry DSN rather than an attacker endpoint, so this is a defense-in-depth containment gap.
Identified by Warden security-review · G88-YYA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds Session Replay product support to the
nativebackend. When a downstream SDK (e.g. sentry-unreal) has staged a recorded clip on disk, the crash daemon now builds areplay_videoenvelope from it and performs an upload so the clip becomes a first-class Session Replay in Sentry, linked to the crash instead of only being available as a file attachment.Key Changes
sentry__session_replay_flush_pending(): scans<database>/replays/for embedder-staged clips (areplay-<id>.jsonmetadata sidecar + its.mp4), builds thereplay_event+replay_recording+ video into areplay_videoenvelope, and hands it to the transport.trace_id, read back from<run>/__sentry-event. This makes the replay carry the same tags as the crash and correlate via the same trace.flush_pendingout-of-process, so the replay is sent in the same session as the crash (no next-launch step).replays/convention.Known Limitations
crashpad/breakpad/inproc.Related items
#skip-changelog