Skip to content

Commit 299fc13

Browse files
committed
Fix code injection in cwd
This isn't a vulnerability since this command is executed inside a container with a unprivileged user. I have replaced the usage of manual `cd` with using the `workdir` option from docker. This option doesn't support env vars expansion (which is safer), so I had to introduce a new setting.
1 parent 077aad5 commit 299fc13

File tree

3 files changed

+8
-8
lines changed

3 files changed

+8
-8
lines changed

readthedocs/doc_builder/environments.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ class BuildCommand(BuildCommandResultMixin):
7575
:py:class:`readthedocs.builds.models.BuildCommandResult` model.
7676
7777
:param command: string or array of command parameters
78-
:param cwd: current working path for the command
78+
:param cwd: Absolute path used as the current working path for the command.
79+
Defaults to ``RTD_DOCKER_WORKDIR``.
7980
:param shell: execute command in shell, default=False
8081
:param environment: environment variables to add to environment
8182
:type environment: dict
@@ -102,7 +103,7 @@ def __init__(
102103
):
103104
self.command = command
104105
self.shell = shell
105-
self.cwd = cwd or '$HOME'
106+
self.cwd = cwd or settings.RTD_DOCKER_WORKDIR
106107
self.user = user or settings.RTD_DOCKER_USER
107108
self.environment = environment.copy() if environment else {}
108109
if 'PATH' in self.environment:
@@ -152,9 +153,7 @@ def run(self):
152153
proc = subprocess.Popen(
153154
command,
154155
shell=self.shell,
155-
# This is done here for local builds, but not for docker,
156-
# as we want docker to expand inside the container
157-
cwd=os.path.expandvars(self.cwd),
156+
cwd=self.cwd,
158157
stdin=None,
159158
stdout=subprocess.PIPE,
160159
stderr=subprocess.STDOUT,
@@ -300,6 +299,7 @@ def run(self):
300299
cmd=self.get_wrapped_command(),
301300
environment=self.environment,
302301
user=self.user,
302+
workdir=self.cwd,
303303
stdout=True,
304304
stderr=True,
305305
)
@@ -360,8 +360,7 @@ def get_wrapped_command(self):
360360
])
361361
)
362362
return (
363-
"/bin/sh -c 'cd {cwd} && {prefix}{cmd}'".format(
364-
cwd=self.cwd,
363+
"/bin/sh -c '{prefix}{cmd}'".format(
365364
prefix=prefix,
366365
cmd=command,
367366
)

readthedocs/doc_builder/python_environments.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ def setup_base(self):
323323
# Don't use virtualenv bin that doesn't exist yet
324324
bin_path=None,
325325
# Don't use the project's root, some config files can interfere
326-
cwd='$HOME',
326+
cwd=None,
327327
)
328328

329329
def install_core_requirements(self):

readthedocs/settings/base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ def TEMPLATES(self):
441441
# https://docs.docker.com/engine/reference/run/#user
442442
RTD_DOCKER_USER = 'docs:docs'
443443
RTD_DOCKER_SUPER_USER = 'root:root'
444+
RTD_DOCKER_WORKDIR = '/home/docs/'
444445

445446
RTD_DOCKER_COMPOSE = False
446447

0 commit comments

Comments
 (0)