Action does not validate that the `SLACK_WEBHOOK_URL` is a valid Slack webhook URL

[nikwen]

The GitHub action does not validate that the passed Slack webhook URL is a valid Slack webhook URL. Thus, an attacker can pass URLs that do not have the format https://hooks.slack.com/*.

If an attacker attacks a GitHub workflow which uses the Slack GitHub action and manages to set the SLACK_WEBHOOK_URL environment variable, they can make the Slack GitHub action send requests to any arbitrary URL (incl. servers on internal networks, or ports on the same machine). That's classic SSRF.

The Slack GitHub action should validate that the passed webhook URL is a valid webhook URL, i.e., has the format https://hooks.slack.com/*.

This is where the environment variable is being read:

slack-github-action/src/config.js

Line 121 in 9a2e0ee

core.getInput("webhook") || process.env.SLACK_WEBHOOK_URL || null,

This is where the request is being sent, with no validation of the webhook URL:

slack-github-action/src/webhook.js

Line 26 in 9a2e0ee

config.inputs.webhook,

I tried reporting this vulnerability responsibly through Slack's HackerOne program, but was told that this was not a security vulnerability. I still believe it should be fixed. Hence, this GitHub issue.

---

As an aside, similar attacks might (or might not) be possible for the API URL or the HTTPS proxy (I haven't had time to look into those):

slack-github-action/src/config.js

Lines 106 to 117 in 9a2e0ee

	api: core.getInput("api"),
	errors: core.getBooleanInput("errors"),
	method: core.getInput("method"),
	payload: core.getInput("payload"),
	payloadDelimiter: core.getInput("payload-delimiter"),
	payloadFilePath: core.getInput("payload-file-path"),
	payloadTemplated: core.getBooleanInput("payload-templated") || false,
	proxy:
	core.getInput("proxy") ||
	process.env.HTTPS_PROXY ||
	process.env.https_proxy ||
	null,

[zimeg]

👋 Hey @nikwen! Thanks for writing in and sharing these findings 👾 ✨

I understand that we're not checking a specific format for webhook URLs but am inclined to call this intentional behavior alongside the flexible api option:

In certain circumstances, such as testing the sent payload, a custom API URL can be used to change where method requests are sent

🔗 https://docs.slack.dev/tools/slack-github-action/additional-configurations#sending-to-a-custom-api-url

We might call out GovSlack as another reason to remain unopinionated about the URL provided IIRC.

The concerns of environment variables overriding expected values might be mitigated with how the action handles input. In this action documentation we recommend using secrets instead of environment variables for both token and webhook values and we attempt to call a provided method before the webhook option is taken:

slack-github-action/src/send.js

Lines 25 to 38 in 9a2e0ee

	/**
	* Perform the posting action of this workflow with configured settings.
	* @param {Config} config
	*/
	async function post(config) {
	switch (true) {
	case !!config.inputs.method:
	return await new Client().post(config);
	case !!config.inputs.webhook:
	return await new Webhook().post(config);
	default:
	throw new SlackError(config.core, "No technique given to post content");
	}
	}

🔬 I'm curious to improve the security of this action as well and wonder if removing support for environment variables might be a practical defense to reduce possible exploits?

  - uses: slackapi/slack-github-action@v3.0.3
    with:
      webhook-type: webhook-trigger
+     webhook: ${{ env.SLACK_WEBHOOK_URL }}
      payload: |
        number: "2"
    env:
      SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}