Slack webhooks: Validate webhook signatures and payload fields at the edge

api.wordpress.org/public_html/dotorg/slack/announce.php

@@ -1,13 +1,21 @@
 <?php
-
-namespace {
-	require dirname( dirname( __DIR__ ) ) . '/includes/hyperdb/bb-10-hyper-db.php';
-	require dirname( dirname( __DIR__ ) ) . '/includes/slack-config.php';
-}
-
-namespace Dotorg\Slack\Announce {
-
-require dirname( dirname( __DIR__ ) ) . '/includes/slack/announce/lib.php';
+/**
+ * Slack slash-command handler for making announcements in channels.
+ *
+ * This is a standalone Slack slash-command handler: WordPress is not loaded, so request
+ * data is never slashed. Slack authenticates itself with one of the shared
+ * `WEBHOOK_TOKEN_*` secrets below; nonces don't exist in server-to-server webhooks.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.MissingUnslash
+ *
+ * @package WordPressdotorg\API\Slack
+ */
+
+namespace Dotorg\Slack\Announce;
+
+require dirname( __DIR__, 2 ) . '/includes/hyperdb/bb-10-hyper-db.php';
+require dirname( __DIR__, 2 ) . '/includes/slack-config.php';
+require dirname( __DIR__, 2 ) . '/includes/slack/announce/lib.php';
 
 function get_avatar( $username, $slack_id, $team_id ) {
 	global $wpdb;
@@ -26,13 +34,14 @@ function get_avatar( $username, $slack_id, $team_id ) {
 	return sprintf( 'https://secure.gravatar.com/avatar/%s?s=96d=mm&r=G&%s', $hash, time() );
 }
 
+if ( ! is_string( $_POST['token'] ?? null ) ) {
+	return;
+}
+
 $i = 0;
 // WEBHOOK_TOKEN_1, WEBHOOK_TOKEN_2, etc.
 while ( defined( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . ++$i ) ) {
 	if ( hash_equals( constant( __NAMESPACE__ . '\\WEBHOOK_TOKEN_' . $i ), $_POST['token'] ) ) {
 		run( $_POST );
 	}
 }
-
-}
-

api.wordpress.org/public_html/dotorg/slack/committers.php

@@ -1,19 +1,33 @@
 <?php
+/**
+ * Slack outgoing-webhook handler for the committers channel.
+ *
+ * This is a standalone Slack outgoing-webhook handler: WordPress is not loaded, so request data
+ * is never slashed. Slack authenticates itself with the shared `WEBHOOK_TOKEN` below; nonces
+ * don't exist in server-to-server webhooks.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.MissingUnslash
+ *
+ * @package WordPressdotorg\API\Slack
+ */
 
 // Allow committers to publicly mention other committers via @committers.
 
 namespace Dotorg\Slack\Committers;
 
 require dirname( dirname( __DIR__ ) ) . '/includes/slack-config.php';
 
-if ( ! hash_equals( WEBHOOK_TOKEN, $_POST['token'] ) ) {
+if ( ! is_string( $_POST['token'] ?? null ) || ! hash_equals( WEBHOOK_TOKEN, $_POST['token'] ) ) {
 	return;
 }
 
+// The Slack user name of whoever triggered the webhook, echoed back in the JSON response below.
+$user_name = (string) filter_var( $_POST['user_name'] ?? '', FILTER_UNSAFE_RAW, FILTER_FLAG_STRIP_LOW );
+
 echo json_encode( array(
 	'username'   => 'wordpressdotorg',
 	'link_names' => 1,
-	'text'       => sprintf( '@%s: Use the `/committers` command.', $_POST['user_name'] ),
+	'text'       => sprintf( '@%s: Use the `/committers` command.', $user_name ),
 ) );
 
 exit;

api.wordpress.org/public_html/dotorg/slack/community-deputies-calendly-webhook.php

@@ -1,4 +1,15 @@
 <?php
+/**
+ * Calendly webhook handler for community deputy meetings.
+ *
+ * Calendly authenticates itself with the shared `COMMUNITY_CALENDLY_SECRET` passed in the webhook
+ * URL, verified below; nonces don't exist in server-to-server webhooks.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification
+ *
+ * @package WordPressdotorg\API\Slack
+ */
+
 namespace WordPressdotorg\API\Slack\Community_Deputy_Webhook;
 use Dotorg\Slack\Send;
 use DateTime, DateTimeZone;
@@ -40,6 +51,7 @@ function api_request( $url ) {
 		trigger_error(
 			'The Calendly token has probably been revoked, the password was probably changed.' .
 			'Please update the COMMUNITY_CALENDLY_TOKEN secrets constant with a new PAT created on https://calendly.com/integrations/api_webhooks from the WordCamp Calendly account.' .
+			// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Plain-text PHP error log entry, not HTTP output.
 			wp_remote_retrieve_body( $req ),
 			E_USER_WARNING
 		);
@@ -49,7 +61,7 @@ function api_request( $url ) {
 }
 
 // Check the request is valid.
-if ( empty( $_GET['secret'] ) || ! hash_equals( COMMUNITY_CALENDLY_SECRET, $_GET['secret'] ) ) {
+if ( empty( $_GET['secret'] ) || ! is_string( $_GET['secret'] ) || ! hash_equals( COMMUNITY_CALENDLY_SECRET, wp_unslash( $_GET['secret'] ) ) ) {
 	header( 'HTTP/1.1 403 Forbidden' );
 	die( 'Invalid secret provided.' );
 }

api.wordpress.org/public_html/dotorg/slack/props.php

@@ -40,6 +40,7 @@
 
 			header( 'X-Slack-No-Retry', 1 ); // Don't retry this event again.
 			trigger_error(
+				// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Plain-text PHP error log entry, not HTTP output.
 				sprintf( 'Received retry for %s because: %s', $message_id, $headers['X-Slack-Retry-Reason'] ),
 				E_USER_NOTICE
 			);
@@ -51,6 +52,7 @@
 		}
 
 	} catch ( Exception $exception ) {
+		// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- Plain-text PHP error log entry, not HTTP output.
 		trigger_error( $exception->getMessage(), E_USER_WARNING );
 
 	} finally {
@@ -59,6 +61,7 @@
 		 * request, so this should still be 200 even if something goes wrong on our end.
 		 */
 		http_response_code( 200 );
+		// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- plain-text response body for Slack.
 		die( $result );
 	}
 }

api.wordpress.org/public_html/dotorg/slack/security-team.php

@@ -1,14 +1,23 @@
 <?php
+/**
+ * Slack security team membership helpers and Trac-facing API.
+ *
+ * Standalone when requested directly: it loads HyperDB but not WordPress, so nothing is
+ * slashed. (Also included as a library by trac/mentions-handler.php, which skips the
+ * request handling below.) The Trac server authenticates with the shared API_TOKEN secret.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.MissingUnslash
+ *
+ * @package WordPressdotorg\API\Slack
+ */
 
-namespace {
-	if ( ! isset( $GLOBALS['wpdb'] ) ) {
-		require dirname( dirname( __DIR__ ) ) . '/includes/hyperdb/bb-10-hyper-db.php';
-	}
-}
+namespace Dotorg\Slack\Security_Team;
 
-namespace Dotorg\Slack\Security_Team {
+if ( ! isset( $GLOBALS['wpdb'] ) ) {
+	require dirname( __DIR__, 2 ) . '/includes/hyperdb/bb-10-hyper-db.php';
+}
 
-require dirname( dirname( __DIR__ ) ) . '/includes/slack-config.php';
+require dirname( __DIR__, 2 ) . '/includes/slack-config.php';
 
 function slack_api( $method, $content = array() ) {
 	$content['token'] = SLACK_TOKEN;
@@ -41,7 +50,7 @@ function get_security_team( $user_field = 'user_login' ) {
 
 	$slack_user_ids = $group['group']['members'];
 	$slack_user_ids = array_filter( $slack_user_ids, function( $user_id ) {
-		return (bool) preg_match( '/^U[A-Z0-9]+$/', $user_id );
+		return (bool) preg_match( '/^U[A-Z0-9]+\z/', $user_id );
 	});
 	$slack_user_ids_for_sql = "'" . implode( "', '", $slack_user_ids ) . "'";
 	$user_ids = $wpdb->get_col( "SELECT user_id FROM slack_users WHERE slack_id IN ($slack_user_ids_for_sql)" );
@@ -62,7 +71,7 @@ function api_call() {
 	header( 'Content-type: text/plain' );
 
 	// Confirm it came from the Trac server.
-	if ( ! hash_equals( API_TOKEN, $_GET['token'] ?? '' ) ) {
+	if ( ! is_string( $_GET['token'] ?? null ) || ! hash_equals( API_TOKEN, $_GET['token'] ) ) {
 		exit;
 	}
 
@@ -76,12 +85,12 @@ function api_call() {
 		exit;
 	}
 
-	echo implode( "\n", $team ) . "\n"; // Trailing newline critical.
+	// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- plain-text list; trailing newline critical.
+	echo implode( "\n", $team ) . "\n";
 	exit;
 }
 
+// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- substring comparison only.
 if ( isset( $_SERVER['REQUEST_URI'] ) && false !== strpos( $_SERVER['REQUEST_URI'], '/security-team.php?token=' ) ) {
 	api_call();
 }
-
-}

api.wordpress.org/public_html/dotorg/slack/subgroup.php

@@ -1,4 +1,16 @@
 <?php
+/**
+ * Slack /subgroup app: slash command and interactivity endpoint.
+ *
+ * This is a standalone Slack app endpoint: only the object cache is loaded, not WordPress, so
+ * request data is never slashed. Slack authenticates every request with an HMAC signature over the
+ * raw request body, verified in `verify_slack_signature()` before anything is dispatched; nonces
+ * don't exist in server-to-server webhooks.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification, WordPress.Security.ValidatedSanitizedInput.MissingUnslash
+ *
+ * @package WordPressdotorg\API\Slack
+ */
 
 namespace Dotorg\Slack\Subgroup;
 
@@ -83,8 +95,27 @@ function ack_and_finish() {
 }
 
 function verify_slack_signature( $body ) {
-	$timestamp = $_SERVER['HTTP_X_SLACK_REQUEST_TIMESTAMP'] ?? '';
-	$signature = $_SERVER['HTTP_X_SLACK_SIGNATURE'] ?? '';
+	// A Unix timestamp, and a `v0=` prefixed HMAC-SHA256 hex digest. Anything else can't be from Slack.
+	$timestamp = filter_var(
+		$_SERVER['HTTP_X_SLACK_REQUEST_TIMESTAMP'] ?? '',
+		FILTER_VALIDATE_REGEXP,
+		[
+			'options' => [
+				'regexp'  => '/^\d+\z/',
+				'default' => '',
+			],
+		]
+	);
+	$signature = filter_var(
+		$_SERVER['HTTP_X_SLACK_SIGNATURE'] ?? '',
+		FILTER_VALIDATE_REGEXP,
+		[
+			'options' => [
+				'regexp'  => '/^v0=[0-9a-f]{64}\z/',
+				'default' => '',
+			],
+		]
+	);
 	if ( ! $timestamp || ! $signature ) {
 		return false;
 	}
@@ -104,16 +135,36 @@ function verify_slack_signature( $body ) {
 
 // Dispatch: slash command vs. interactivity callback.
 if ( isset( $_POST['payload'] ) ) {
+	// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- JSON document covered by the verified signature above; fields are validated where read.
 	$payload = json_decode( $_POST['payload'], true );
 	handle_interaction( $payload );
 	exit;
 }
 handle_slash_command();
 
 function handle_slash_command() {
-	$channel_id = $_POST['channel_id'] ?? '';
-	$user_id    = $_POST['user_id'] ?? '';
-	$trigger_id = $_POST['trigger_id'] ?? '';
+	// Slack channel and user IDs are uppercase alphanumeric, eg. `C0123ABCD` and `U0123ABCD`.
+	$id_options = [
+		'options' => [
+			'regexp'  => '/^[A-Z0-9]+\z/',
+			'default' => '',
+		],
+	];
+
+	$channel_id = (string) filter_var( $_POST['channel_id'] ?? '', FILTER_VALIDATE_REGEXP, $id_options );
+	$user_id    = (string) filter_var( $_POST['user_id'] ?? '', FILTER_VALIDATE_REGEXP, $id_options );
+
+	// A trigger ID, eg. `13345224609.738474920.8088930838d88f008e0`.
+	$trigger_id = (string) filter_var(
+		$_POST['trigger_id'] ?? '',
+		FILTER_VALIDATE_REGEXP,
+		[
+			'options' => [
+				'regexp'  => '/^[A-Za-z0-9.]+\z/',
+				'default' => '',
+			],
+		]
+	);
 
 	// trigger_id is only valid for 3s, and listing every subgroup's membership will take
 	// longer than that. Open a loading view now, then views.update after we have the data.
@@ -692,7 +743,7 @@ function finalize_create( $new_id, $name, $creator, $parent_id, $parent_name, $u
 	// conversations.invite fails the entire batch on one malformed ID. Filter
 	// defensively so a stray character in a config value can't sink everyone.
 	$valid_invitees = array_values( array_filter( $invitees, function ( $id ) {
-		return preg_match( '/^[UWB][A-Z0-9]+$/', $id );
+		return preg_match( '/^[UWB][A-Z0-9]+\z/', $id );
 	} ) );
 	$invitees = $valid_invitees;
 

api.wordpress.org/public_html/dotorg/slack/trac-bot.php

@@ -1,4 +1,14 @@
 <?php
+/**
+ * Slack outgoing-webhook Trac bot: expands ticket and changeset references.
+ *
+ * Slack authenticates itself with the shared `URL_SECRET__TRAC_BOT` secret passed in the
+ * outgoing webhook URL, verified below; nonces don't exist in server-to-server webhooks.
+ *
+ * phpcs:disable WordPress.Security.NonceVerification
+ *
+ * @package WordPressdotorg\API\Slack
+ */
 
 namespace {
 	require dirname( dirname( __DIR__ ) ) . '/wp-init.php';
@@ -11,12 +21,20 @@
 namespace Dotorg\Slack\Trac {
 
 	// Verify it came from Slack.
-	if ( ! hash_equals( URL_SECRET__TRAC_BOT, $_GET['token'] ?? '' ) ) {
+	if ( ! is_string( $_GET['token'] ?? null ) || ! hash_equals( URL_SECRET__TRAC_BOT, wp_unslash( $_GET['token'] ) ) ) {
 		return;
 	}
 
+	/*
+	 * The Slack channel name, user name, and message timestamp are interpolated into the Trac
+	 * comment and the message permalink below.
+	 */
+	$channel_name  = sanitize_text_field( wp_unslash( $_POST['channel_name'] ?? '' ) );
+	$user_name     = sanitize_text_field( wp_unslash( $_POST['user_name'] ?? '' ) );
+	$msg_timestamp = sanitize_text_field( wp_unslash( $_POST['timestamp'] ?? '' ) );
+
 	// Prevent recursion.
-	if ( $_POST['user_name'] === 'slackbot' ) {
+	if ( 'slackbot' === $user_name ) {
 		return;
 	}
 
@@ -99,7 +117,7 @@
 
 		$slack->send( $parser->get_channel(), $parser->get_thread() );
 
-		if ( $_POST['channel_name'] === 'test' ) {
+		if ( 'test' === $channel_name ) {
 			// Don't post to Trac if we're coming from #test.
 			continue;
 		}
@@ -111,7 +129,7 @@
 
 		$trac_xmlrpc = new \Trac( 'slackbot', SLACKBOT_WPORG_PASSWORD, "https://$trac.trac.wordpress.org/login/xmlrpc" );
 
-		$comment = sprintf( $comment_template, $_POST['channel_name'], $_POST['user_name'], str_replace( '.', '', $_POST['timestamp'] ) );
+		$trac_comment = sprintf( $comment_template, $channel_name, $user_name, str_replace( '.', '', $msg_timestamp ) );
 		foreach ( $results['ticket'] as $ticket ) {
 			$ticket_id = is_array( $ticket ) ? $ticket['id'] : $ticket;
 
@@ -135,7 +153,7 @@
 
 			$parser->set_redundancy( 'trac', $trac, 'ticket', $ticket_id );
 
-			$trac_xmlrpc->ticket_update( $ticket_id, $comment );
+			$trac_xmlrpc->ticket_update( $ticket_id, $trac_comment );
 		}
 	}
 }