A heretofore undiscovered (and unmitigated) exploit that allows an attacker to write to a region of memory may allow setting one but not the other.

In D59130#1360721, @markj wrote:

In D59130#1360708, @dteske wrote:

Context.

All my seceng colleagues point out that unquoted parameter expansion with the unintended consequences of glob expansion is a lack of defense-in-depth architecture with security implications.

In that context, this is very much a security bug.

What context are you referring to?

In D59130#1360708, @dteske wrote:

Context.

All my seceng colleagues point out that unquoted parameter expansion with the unintended consequences of glob expansion is a lack of defense-in-depth architecture with security implications.

In that context, this is very much a security bug.

In D59130#1360663, @markj wrote:

In D59130#1360660, @dteske wrote:

In D59130#1360640, @markj wrote:

In D59130#1360422, @dteske wrote:

In D59130#1356525, @markj wrote:

What security problem is being fixed here?

Denial-of-Service.

A poisoned LOCALBASE value such as:

LOCALBASE='/*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/*'

NB: That's 5x (10x /*)

Causes massive disk I/O and only in the minority of cases with little-to-no filesystem contents does it only result in a minor inconvenience (sub 30 second delay in launch) whereas on moderately to substantially loaded filesystems (with many files) does it in-turn result in never getting passed the : ${LOCALBASE:=...} line.

What's going on when you don't quote the parameter expansion, is that globs get expanded. Yes, it's true that : throws those expanded arguments away, but the damage is done in the I/O and compute required to expand those globs.

Quoting the parameter expansion (as-in : "${LOCALBASE:=...}" prevents any globs stuffed into a poisoned LOCALBASE rendered inert.

Literally ShellCheck's SC2223 DoS-via-glob warning.

If someone can modify your environment variables, you're in a bad place already, e.g., if they point ARC_CMD to a script of their choosing.

While imaginative (injecting a 3rd party into the mix via "someone") that's not exactly the concern.

The word "poisoned" similarly evokes a third party.

In D59130#1360660, @dteske wrote:

In D59130#1360640, @markj wrote:

In D59130#1360422, @dteske wrote:

In D59130#1356525, @markj wrote:

What security problem is being fixed here?

Denial-of-Service.

A poisoned LOCALBASE value such as:

LOCALBASE='/*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/*'

NB: That's 5x (10x /*)

Causes massive disk I/O and only in the minority of cases with little-to-no filesystem contents does it only result in a minor inconvenience (sub 30 second delay in launch) whereas on moderately to substantially loaded filesystems (with many files) does it in-turn result in never getting passed the : ${LOCALBASE:=...} line.

What's going on when you don't quote the parameter expansion, is that globs get expanded. Yes, it's true that : throws those expanded arguments away, but the damage is done in the I/O and compute required to expand those globs.

Quoting the parameter expansion (as-in : "${LOCALBASE:=...}" prevents any globs stuffed into a poisoned LOCALBASE rendered inert.

Literally ShellCheck's SC2223 DoS-via-glob warning.

If someone can modify your environment variables, you're in a bad place already, e.g., if they point ARC_CMD to a script of their choosing.

While imaginative (injecting a 3rd party into the mix via "someone") that's not exactly the concern.

In D59130#1360640, @markj wrote:

In D59130#1360422, @dteske wrote:

In D59130#1356525, @markj wrote:

What security problem is being fixed here?

Denial-of-Service.

A poisoned LOCALBASE value such as:

LOCALBASE='/*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/*'

NB: That's 5x (10x /*)

Causes massive disk I/O and only in the minority of cases with little-to-no filesystem contents does it only result in a minor inconvenience (sub 30 second delay in launch) whereas on moderately to substantially loaded filesystems (with many files) does it in-turn result in never getting passed the : ${LOCALBASE:=...} line.

What's going on when you don't quote the parameter expansion, is that globs get expanded. Yes, it's true that : throws those expanded arguments away, but the damage is done in the I/O and compute required to expand those globs.

Quoting the parameter expansion (as-in : "${LOCALBASE:=...}" prevents any globs stuffed into a poisoned LOCALBASE rendered inert.

Literally ShellCheck's SC2223 DoS-via-glob warning.

If someone can modify your environment variables, you're in a bad place already, e.g., if they point ARC_CMD to a script of their choosing.

In D59130#1360422, @dteske wrote:

In D59130#1356525, @markj wrote:

What security problem is being fixed here?

Denial-of-Service.

A poisoned LOCALBASE value such as:

LOCALBASE='/*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/* /*/*/*/*/*/*/*/*/*/*'

NB: That's 5x (10x /*)

Causes massive disk I/O and only in the minority of cases with little-to-no filesystem contents does it only result in a minor inconvenience (sub 30 second delay in launch) whereas on moderately to substantially loaded filesystems (with many files) does it in-turn result in never getting passed the : ${LOCALBASE:=...} line.

What's going on when you don't quote the parameter expansion, is that globs get expanded. Yes, it's true that : throws those expanded arguments away, but the damage is done in the I/O and compute required to expand those globs.

Quoting the parameter expansion (as-in : "${LOCALBASE:=...}" prevents any globs stuffed into a poisoned LOCALBASE rendered inert.

Literally ShellCheck's SC2223 DoS-via-glob warning.

In D59130#1356525, @markj wrote:

What security problem is being fixed here?

Refactor. While considering the value of checking for -h at the sub-
command level versus globally, it became clear that we could do one
better. Checking for -h globally is woefully incongruent with the
given sub-command's option-flags. To be concise: one cannot know
whether -h is a true request for usage or something that by POSIX
standards should be considered an argument. To avoid having to
teach the global check about all the possible getopts variations of
each sub-command, it was deemed most-appropriate to allow handing-
off the actual usage check to each sub-command. However, that left-
open the quagmire that was prematurely sourcing git-sh-setup where
I believe it to be much more correct to only source that (and check
for external dependencies) after one has determined that the user
has not requested the usage statement. Generally speaking, the user
should be able to get the usage statement without having to first
satisfy every dependency (wherein git-sh-setup is also considered
another dependency).

What security problem is being fixed here?

thanks! @markj what do you think?

Same but for man-page

Only introduce -t

Advertise -h in all sub-commands.

Fix synopsis to align with man-page/code. Refactor.

In D59019#1354277, @adrian wrote:

devin, let's split the 'handle -h and provide better help' change from the tag handling change.

devin, let's split the 'handle -h and provide better help' change from the tag handling change.