From 185082b720e654786d052b53560abd0805ccd04d Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Sun, 15 Dec 2024 20:04:06 +0000 Subject: [PATCH] fix(ingest): sanitise token (#26918) --- .../event-pipeline/populateTeamDataStep.ts | 4 ++++ rust/capture/src/token.rs | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/plugin-server/src/worker/ingestion/event-pipeline/populateTeamDataStep.ts b/plugin-server/src/worker/ingestion/event-pipeline/populateTeamDataStep.ts index de63033393f46..042eb197122a3 100644 --- a/plugin-server/src/worker/ingestion/event-pipeline/populateTeamDataStep.ts +++ b/plugin-server/src/worker/ingestion/event-pipeline/populateTeamDataStep.ts @@ -2,6 +2,7 @@ import { PluginEvent } from '@posthog/plugin-scaffold' import { eventDroppedCounter } from '../../../main/ingestion-queues/metrics' import { PipelineEvent } from '../../../types' +import { sanitizeString } from '../../../utils/db/utils' import { UUID } from '../../../utils/utils' import { captureIngestionWarning } from '../utils' import { tokenOrTeamPresentCounter } from './metrics' @@ -42,6 +43,9 @@ export async function populateTeamDataStep( } else if (event.team_id) { team = await runner.hub.teamManager.fetchTeam(event.team_id) } else if (event.token) { + // HACK: we've had null bytes end up in the token in the ingest pipeline before, for some reason. We should try to + // prevent this generally, but if it happens, we should at least simply fail to lookup the team, rather than crashing + event.token = sanitizeString(event.token) team = await runner.hub.teamManager.getTeamByToken(event.token) } diff --git a/rust/capture/src/token.rs b/rust/capture/src/token.rs index 7924cc9511485..3e7d99c333a7f 100644 --- a/rust/capture/src/token.rs +++ b/rust/capture/src/token.rs @@ -12,6 +12,7 @@ pub enum InvalidTokenReason { TooLong, NotAscii, PersonalApiKey, + NullByte, } impl InvalidTokenReason { @@ -22,6 +23,7 @@ impl InvalidTokenReason { // Self::IsNotString => "not_string", Self::TooLong => "too_long", Self::PersonalApiKey => "personal_api_key", + Self::NullByte => "null_byte", } } } @@ -57,6 +59,11 @@ pub fn validate_token(token: &str) -> Result<(), InvalidTokenReason> { return Err(InvalidTokenReason::PersonalApiKey); } + // We refuse tokens with null bytes + if token.contains('\0') { + return Err(InvalidTokenReason::NullByte); + } + Ok(()) } @@ -96,4 +103,12 @@ mod tests { assert!(valid.is_err()); assert_eq!(valid.unwrap_err(), InvalidTokenReason::PersonalApiKey); } + + #[test] + fn blocks_null_byte() { + let valid = validate_token("hello\0there"); + + assert!(valid.is_err()); + assert_eq!(valid.unwrap_err(), InvalidTokenReason::NullByte); + } }