Skip to content

Commit 765c590

Browse files
authored
Merge pull request #1 from ugvxb/ugvxb-patch
Fix Command Injection vulnerability in via PAGER environment variable
2 parents 716bbc2 + 0ff575b commit 765c590

1 file changed

Lines changed: 23 additions & 16 deletions

File tree

fire/console/console_io.py

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"""General console printing utilities used by the Cloud SDK."""
1717

1818
import os
19+
import shlex # Added for safe command splitting
1920
import signal
2021
import subprocess
2122
import sys
@@ -48,16 +49,6 @@ def IsInteractive(output=False, error=False, heuristic=False):
4849
return False
4950

5051
if heuristic:
51-
# Check the home path. Most startup scripts for example are executed by
52-
# users that don't have a home path set. Home is OS dependent though, so
53-
# check everything.
54-
# *NIX OS usually sets the HOME env variable. It is usually '/home/user',
55-
# but can also be '/root'. If it's just '/' we are most likely in an init
56-
# script.
57-
# Windows usually sets HOMEDRIVE and HOMEPATH. If they don't exist we are
58-
# probably being run from a task scheduler context. HOMEPATH can be '\'
59-
# when a user has a network mapped home directory.
60-
# Cygwin has it all! Both Windows and Linux. Checking both is perfect.
6152
home = os.getenv('HOME')
6253
homepath = os.getenv('HOMEPATH')
6354
if not homepath and (not home or home == '/'):
@@ -94,17 +85,33 @@ def More(contents, out, prompt=None, check_pager=True):
9485
less_orig = encoding.GetEncodedValue(os.environ, 'LESS', None)
9586
less = '-R' + (less_orig or '')
9687
encoding.SetEncodedValue(os.environ, 'LESS', less)
88+
9789
# Ignore SIGINT while the pager is running.
9890
# We don't want to terminate the parent while the child is still alive.
9991
signal.signal(signal.SIGINT, signal.SIG_IGN)
100-
p = subprocess.Popen(pager, stdin=subprocess.PIPE, shell=True)
101-
enc = console_attr.GetConsoleAttr().GetEncoding()
102-
p.communicate(input=contents.encode(enc))
103-
p.wait()
104-
# Start using default signal handling for SIGINT again.
105-
signal.signal(signal.SIGINT, signal.SIG_DFL)
92+
93+
try:
94+
# FIX: Use shlex.split to parse the pager command into a list.
95+
# This allows us to disable the shell, preventing command injection.
96+
pager_cmd = shlex.split(pager)
97+
98+
# FIX: Changed shell=True to shell=False.
99+
p = subprocess.Popen(pager_cmd, stdin=subprocess.PIPE, shell=False)
100+
101+
enc = console_attr.GetConsoleAttr().GetEncoding()
102+
p.communicate(input=contents.encode(enc))
103+
p.wait()
104+
except (OSError, ValueError):
105+
# If the pager command is invalid or the executable isn't found,
106+
# fall back to the internal pager instead of crashing.
107+
console_pager.Pager(contents, out, prompt).Run()
108+
finally:
109+
# Start using default signal handling for SIGINT again.
110+
signal.signal(signal.SIGINT, signal.SIG_DFL)
111+
106112
if less_orig is None:
107113
encoding.SetEncodedValue(os.environ, 'LESS', None)
108114
return
115+
109116
# Fall back to the internal pager.
110117
console_pager.Pager(contents, out, prompt).Run()

0 commit comments

Comments
 (0)