Powerlevel9k: Fixing Double Expansion

Created on 19 Apr 2017  路  8Comments  路  Source: Powerlevel9k/powerlevel9k

Okay, so @belak was kind enough to fix #445 with PR #486, but unfortunately that broke prompts for a bunch of folks (#487). I did not experience a prompt breakage when I tried it out, unfortunately.

Fixing this is somewhat urgent, as master is now once again susceptible to the pw3nage exploit.

From @belak's comments in #486:

Because PROMPT and RPROMPT are in double quotes, they expand the shell commands inside them. Because prompt_subst is set, whatever these commands expand to will also be expanded when the prompt is displayed.

This fixes the problem by using single quotes, ensuring that they are only expanded once, on prompt display.

So the reason some of us were seeing the exploit fire in #445 but not others is because of this prompt_subst setting. What isn't clear to me is how we fix the potential exploit within P9k without breaking prompt expansion that we rely on.

@dritter

bug master next security

Most helpful comment

I tested this on Linux with antibody, antigen, omz, prezto, prezto-community, vanilla ZSH, zgen, zim, zplug, zplugin, zpm, zulu and on OSX with omz, vanilla ZSH, zplug, zulu. I tested this with and without setting PROMPT_SUBST upfront.
In none of my test I encountered an exploited shell. :)

All 8 comments

I think I might've found a solution; see my PR #493

I think @belak fix is right. Only thing is that it requires to set PROMPT_SUBST beforehand. But this could be done right before we set the PROMPT variable. I have done that in #495

@belak was so kind to fix the options in #496 . I closed #495 in favour of that one.

Okay, #496 has been merged into next. I'd like to have next tested thoroughly at this point before I revert the revert in master. If any of the following are willing to test next to confirm that (a) the pw3nage exploit is resolved and that (b) your prompt doesn't break, that would be excellent:

@Strayer @maddie @qifei9 @sethdeckard @theSoenke @dritter @shibumi @wadkar

Separately, @wadkar, I think this means that #493 is superseded?

Yep, I will close #493 as its not needed. I learnt quite a bit about setopts through this issue. Thanks!

Strange, not sure how this issue got closed - that was not my intention. Sorry.

I tested this on Linux with antibody, antigen, omz, prezto, prezto-community, vanilla ZSH, zgen, zim, zplug, zplugin, zpm, zulu and on OSX with omz, vanilla ZSH, zplug, zulu. I tested this with and without setting PROMPT_SUBST upfront.
In none of my test I encountered an exploited shell. :)

@dritter - Thank you for your thorough testing, as always. With tens of thousands of users at this point, it's critical we remain stable.

Okay, all of these changes are now in master. If you can, I would appreciate everyone trying out master to make sure everything is working normally (i.e., your prompt doesn't break).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dritter picture dritter  路  5Comments

controversial picture controversial  路  4Comments

xufab picture xufab  路  4Comments

guidoilbaldo picture guidoilbaldo  路  5Comments

yoyoys picture yoyoys  路  3Comments