patchThe GNU Bourne-Again SHell - Patches: patch #10496, Don't perform command substitution...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #10496: Don't perform command substitution on username

Submitter:  schongallam <schongallam>
Submitted:  Sun 19 Jan 2025 04:08:57 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Open

Tue 21 Jan 2025 02:17:20 PM UTC, comment #3: 

On 1/21/25 12:40 AM, schongallam wrote:

> Follow-up Comment #2, patch #10496 (group bash):
>
> Thanks for your interest in this.  It seems like one of those things that
> appears kind of absurd today, but could become a future case of "hindsight is
> 20/20" in the case of some novel attack chain.


Sure. It's just a head-shaking moment when you find out a distro has
actually done this. They don't seem to realize what a potential
nightmare it could be for applications that might use the user name,
since including non-alphanumerics breaks assumptions that have been
valid for a good half-century.

  I would like to credit Ben

> Kallus and Jonah Weinbaum with discovering command substitution on malicious
> user names in this case.  I'm drafting a brief, blog-style, write up which
> includes this scenario -- please let me know if you have any concerns.  It
> seems pretty low risk for now, though.


Go for it.

Chet Ramey <chet>
Group administrator
Tue 21 Jan 2025 05:40:08 AM UTC, comment #2: 

Thanks for your interest in this.  It seems like one of those things that appears kind of absurd today, but could become a future case of "hindsight is 20/20" in the case of some novel attack chain.  I would like to credit Ben Kallus and Jonah Weinbaum with discovering command substitution on malicious user names in this case.  I'm drafting a brief, blog-style, write up which includes this scenario -- please let me know if you have any concerns.  It seems pretty low risk for now, though.

schongallam <schongallam>
Mon 20 Jan 2025 07:55:39 PM UTC, comment #1: 


> Date: Sun 19 Jan 2025 04:08:57 AM UTC By: schongallam <schongallam>
> Background: Linux usernames created with --badnames can include names such as
> $(rm -rf ~) or other injected commands.


Good god, what a spectacularly bad idea. There's no chance this will have
any unanticipated negative effects, right?

> Recent discussions on the Debian
> developers listserv have also debated allowing even more diverse character
> sets to be used in the username.  Usernames are typically displayed in the
> command prompt by expansion of '\u'.


Not even going to restrict it to alphanumerics in the primary locale, eh?


> Current behavior: Prompt generation currently performs command substitution on
> the value of \u after expanding it.

 
It performs all the shell word expansions on the prompt string after
decoding escape sequences.

> The treatment of \u currently differs
> from \w and \h, for instance, which are not command substituted.


Yes, you can put arbitrary characters into filenames, and even hostnames
can be altered like that, but nobody had the, um, inspiration to make
usernames like this.

>
> Proposed change:  This patch treats expansion of \u more like \w.


Well, let's start with backslash quoting the characters in the username
to prevent escapades like the above, and go from there. I suppose it's
for the best.

Thanks for the heads-up.

Chet Ramey <chet>
Group administrator
Sun 19 Jan 2025 04:08:57 AM UTC, original submission:  

Background: Linux usernames created with --badnames can include names such as $(rm -rf ~) or other injected commands.  Recent discussions on the Debian developers listserv have also debated allowing even more diverse character sets to be used in the username.  Usernames are typically displayed in the command prompt by expansion of '\u'.

Current behavior: Prompt generation currently performs command substitution on the value of \u after expanding it.  The treatment of \u currently differs from \w and \h, for instance, which are not command substituted.

Proposed change:  This patch treats expansion of \u more like \w.  Specifically, in parse.y, decode_prompt_string(), the username string (as 'temp') is subject to sh_strvis() and sh_backslash_quote_for_double_quotes() before appending it to the 'result' string.

Impact:  This patch mitigates potential command injection and printability issues involving the username (whether incidental or malicious).  This change is beneficial by preventing unexpected code execution or "command not found" errors.  It also ensures the user's name is displayed as intended in the prompt string.

Tested on: Debian 6.1.99-1 (2024-07-15) x86_64; GNU bash, version 5.2.37(1)-release (x86_64-pc-linux-gnu) (master branch)

I hope you find this patch helpful, no worries otherwise.

schongallam <schongallam>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by chet (Posted a comment)
  • -email is unavailable- added by schongallam (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-01-20 chet StatusNone Done
    2025-01-19 schongallam Attached File- Added dont_command_substitute_username.patch, #56780

    Back to the top

    Powered by Savane 3.14-7003.
    Corresponding source code