Discourse_docker: discourse-setup makes sed parse SMTP password

Bug
1

I previously posted this on the GitHub issue tracker which has now been removed (including my post), so I’m posting it here again.

discourse-setup uses sed to place the user provided SMTP password in the configuration file. The user input is used as a regular expression by sed (-e) there, while it is really just a plain text string.

This works as long as the password does not contain any characters which sed would interpret as an expression / command.

When it fails to work, the user may get to see a sed error message in the discourse-setup output, and installation may fail.

This is potentially a security issue, since sed might end up executing the input string (///e).

2

Sorry about that! We never intended to use GitHub Issues, it was left enabled unintentionally…but now you’re in the right place!

2 Likes
3

I was fortunately able to reconstruct my reports from the cache of a web search engine. Since I had previously created an account here on meta.discouse.org the extra effort I had to spend to migrate the reports here was limited. Other people who reported bugs on the GitHub issue tracker may not be able to gather the same motivation to migrate their reports to a separate site (which is not a common bug tracker) which they may not have an account on, yet.

4

Sure @pfaffman can have a peek as he has time.

5

Yeah. That all seems true. It’s fragile that way.

That would require someone who is giving an admin an SMTP password to be evil, but you never know.

I suppose I should do this:

2 Likes
6

Even better might be to not interpret the (potentially untrusted) input as a plain text string, not a regular expression, as discussed at

This is, however, more complex, and depends on the GNU Bash shell (i.e. it is a “bashism”). The latter may not matter, though, since the script already states that it depends on
#!/usr/bin/env bash

7

Did this get addressed @pfaffman?

8

At least mostly. Here’s what read_config looks like now. The sed replacement mostly fixes up stuff that needs to be escaped.

read_config() {
  config_line=`egrep "^  #?$1:" $web_file`
  read_config_result=`echo $config_line | awk  --field-separator=":" '{print $2}'`
  read_config_result=`echo $read_config_result | sed "s/^\([\"']\)\(.*\)\1\$/\2/g"`
}

I think it now works at least with characters likely to be in a password. I don’t think I’ve seen any complaints about wonky passwords breaking discourse-setup in a long while.

I don’t consider this much of a security issue, as it would require either the sysadmin of the Discourse server or the sysadmin of the mail server to be malicious.

2 Likes