Skip to content

Commit 080b31b

Browse files
Fiddle-Config Teamcopybara-github
authored andcommitted
Add allow_imports option to DEFINE_fiddle_config etc., default True
A later change will flip the default value of this flag to False. Setting this flag to False fixes an RCE security vulnerability that occurs if the Fiddle flags come from an untrusted or less-trusted source. It prevents Fiddle from implicitly loading modules and executing code when dotted names are passed, such as --config=config:foo.bar(). PiperOrigin-RevId: 880949323
1 parent 24c1d2e commit 080b31b

File tree

1 file changed

+29
-1
lines changed

1 file changed

+29
-1
lines changed

fiddle/_src/absl_flags/flags.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ def __init__(
135135
# This will save all arguments that have passed, when unparse() is called
136136
# this will also be reset to empty.
137137
self._all_arguments = []
138+
139+
if default_module is None and not allow_imports:
140+
raise ValueError(
141+
"A Fiddle config flag must either specify a default_module to load "
142+
"configs from, or set allow_imports=True to allow them to be loaded "
143+
"from any module."
144+
)
145+
138146
super().__init__(*args, **kwargs)
139147

140148
def _apply_fiddler(self, cfg: config.Buildable, expression: str):
@@ -239,7 +247,21 @@ def value(self):
239247
or command == "config_file"
240248
or command == "config_str"
241249
):
242-
self._parse_config(command, expression)
250+
try:
251+
self._parse_config(command, expression)
252+
except ValueError as e:
253+
if (
254+
"Could not resolve reference to named function" in str(e)
255+
and not self.allow_imports
256+
):
257+
raise ValueError(
258+
"If this is a new error in previously-functional code, pass "
259+
"allow_imports=True to DEFINE_fiddle_config(). This allows "
260+
"anyone with control of the fiddle command-line flags to "
261+
"execute arbitrary code; only do this if such imports are "
262+
"trusted."
263+
) from e
264+
raise
243265

244266
elif command == "set":
245267
utils.set_value(self._value, expression)
@@ -267,6 +289,7 @@ def DEFINE_fiddle_config( # pylint: disable=invalid-name
267289
pyref_policy: Optional[serialization.PyrefPolicy] = None,
268290
flag_values: flags.FlagValues = flags.FLAGS,
269291
required: bool = False,
292+
allow_imports: bool = True,
270293
) -> flags.FlagHolder[Any]:
271294
r"""Declare and define a fiddle command line flag object.
272295
@@ -325,6 +348,10 @@ def main(argv) -> None:
325348
registered. This should almost never need to be overridden.
326349
required: bool, is this a required flag. This must be used as a keyword
327350
argument.
351+
allow_imports: If true, then fully qualified dotted names may be used to
352+
specify configs or fiddlers that should be automatically imported. This
353+
grants arbitrary code execution to anyone who can specify the fiddle
354+
command-line flags.
328355
329356
Returns:
330357
A handle to defined flag.
@@ -338,6 +365,7 @@ def main(argv) -> None:
338365
parser=flags.ArgumentParser(),
339366
serializer=FiddleFlagSerializer(pyref_policy=pyref_policy),
340367
help_string=help_string,
368+
allow_imports=allow_imports,
341369
),
342370
flag_values=flag_values,
343371
required=required,

0 commit comments

Comments
 (0)