dangerous eval's in the shell code

[calestyo]

• I have read through the manual page (man fzf)
• I have the latest version of fzf
• I have searched through the existing issues

Info

• OS
  • Linux
  • Mac OS X
  • Windows
  • Etc.
• Shell
  • bash
  • zsh
  • fish

Problem / Steps to reproduce

Hey.

By chance I've stumbled over the following:

$ ls -al /tmp/FOO
ls: cannot access '/tmp/FOO': No such file or directory

If I now do:

$ ssh -i $(echo PWND > /tmp/FOO)**<TAB>

and then ESC without any selection being made (but I guess it would also happen if a selection is made), one gets the following messed up line on the prompt:

$ ssh -i $(echo PWND > $(echo PWND > /tmp/FOO)** ^C

which I then abort with Ctrl-C, so it's not executed by me pressing Enter, yet:

$ cat /tmp/FOO
PWND
$

The reason for this is likely:

fzf/shell/completion.bash

Line 175 in a3ff49a

eval "base=$base"

Perhaps less of an "attack", but one can "easily" imagine that someone accidentally writes: $(rm -rf / )**<TAB> rather than a safe $(rm -rf / ) **<TAB> (with a space before the **).

Any evals on values which in turn had some expansion/substitution are always dangerous, unless it's made sure that those cannot contain other shell code.

I haven't checked the other evals incompletion.bash yet.

The evals inkey-bindings.bash are IMO safe, because for them it's fully expected by the user, that whatever is set in FZF_ALT_C_COMMAND and friends, is executed.

However, vars like FZF_TMUX_HEIGHT, FZF_DEFAULT_OPTS may also accidentally be used to execute any commands... maybe one should check them for "evil" shell meta-characters (like ;, $ and more) and ignore them if set.

Cheers,
Chris.

[calestyo]

However, vars like FZF_TMUX_HEIGHT, FZF_DEFAULT_OPTS may also accidentally be used to execute any commands... maybe one should check them for "evil" shell meta-characters (like ;, $ and more) and ignore them if set.

That part is bogus... I forgot hat the options are (as far as I can see: always) given via the env var FZF_DEFAULT_OPTS="$opts", so there will be no accidental execution of codes from them.

[calestyo]

As for the evals in completion.bash:

I'd say that:

fzf/shell/completion.bash

Line 154 in a3ff49a

eval "${orig_complete/ -F / -o nospace -F }"

and:

fzf/shell/completion.bash

Line 156 in a3ff49a

eval "$orig_complete"

are IMO safe, as orig_complete is only from controlled input ($(complete -p "$orig_cmd" 2> /dev/null)) and the functions called before (_completion_loader and __fzf_orig_completion) don’t seem to modify it.

fzf/shell/completion.bash

Line 175 in a3ff49a

eval "base=$base"

is unsafe as mentioned before, actually I don’t even understand what it does (I mean I don't understand what it's supposed to achieve).

But printf’s %q might be used to make it save!?

fzf/shell/completion.bash

Line 185 in a3ff49a

matches=$(eval "$1 $(printf %q "$dir")" | FZF_DEFAULT_OPTS="--height ${FZF_TMUX_HEIGHT:-40%} --reverse --scheme=path --bind=ctrl-z:ignore ${FZF_DEFAULT_OPTS-} ${FZF_COMPLETION_OPTS-} $2" __fzf_comprun "$4" -q "$leftover" | while read -r item; do

is AFAICS save, $1 is controlled by us and either _fzf_compgen_path or _fzf_compgen_dir and the rest of the string is already escape via printf’s %q.

TODO: check (it's too late and I don't understand it right now ^^):

fzf/shell/completion.bash

Line 358 in a3ff49a

eval "$def"

[junegunn]

actually I don’t even understand what it does

Without the line, something like ls ~/**<tab> or ls $HOME/**<tab> doesn't work as expected. But I agree that we should try to prevent it from involuntarily running a command. Do you have a better idea?

[calestyo]

Ah, I see.

Well… if it was just for ~ and ${HOME}, one could perhaps try to simply search&replace it - but I assume you want parameter expansion in all forms and for all variables?

One could perhaps try to escape any other expansions/substitutions, but I think that's also difficult to do propely and likely fragile.

Might be easier if only the $VAR and ${VAR} forms were supported, but but as soon as anything like arrays or ${VAR-} and the like are desired, it seems difficult to me.

Using envsubst seems also no real solution to me, a) it would require another fork, b) it would require all these vars to be exported.

[calestyo]

I've just seen that bash-completion provides a number of functions (_tilde(), __expand_tilde_by_ref() and _expand()), but just for tilde expansion.

If you'd say it's enough to support just that, we could simply use the right one from bash-completion, and simply do nothing if it's not available (honestly, who uses fzf’s completion but doesn’t use bash-completion?).

[calestyo]

The more I think about it, the more I'd come to the conclusion that simply only ~/ should be supported, but not other forms of ${HOME} etc. pp.

a) It seems to require a more or less a shell code parser to determine what to expand: ${HOME}/**<Tab> should probably be expanded, but '${HOME}'/**<Tab> probably not.
b) If ${HOME} would be supported for expansion, then - for a really generic solution - why not also $(echo /home/user). But then we inherently get the possibility of dangerous side effects.

What I would actually rather like was, if e.g. ls a/*/b/**<Tab> would be supported, ... but one would need to think how that can be cleanly done (especially ** is also a valid shell pattern) and in which circumstance it would even make sense.

Anyway… would you consider to support only ~/ and use bash-completions function for that (and if not, simply don't expand it)?

[junegunn]

honestly, who uses fzf’s completion but doesn’t use bash-completion?

That's just a guess. We have no data to support this claim. For example, I installed bash-completion only a few years ago after using fzf for several years before that.

Anyway… would you consider to support only ~/ and use bash-completions function for that (and if not, simply don't expand it)?

• I don't like the idea of introducing a dependency.
• We should at least expand $ENV_VAR, ~, and ~other_user_name. I can imagine many users would do something like vim $PROJECT1_HOME/**<tab>.

[calestyo]

That's just a guess.

Arguably ;-)

• I don't like the idea of introducing a dependency.

In my opinion we (from the fzf side) should generally try to keep the users shell environment as clean as possible (i.e. not defining new variable/function names or messing around with bindings or assuming any specific settings for them - unless of course, there's no way around).

This in turn means, that we should not define our own e.g. _fzf_tilde(), in which case a user would have similar functions in his shell environment if he uses both, fzf and bash-completion.

So in the end I think we can do the following:

• Either "depend" on bash-completion (but fail gracefully, in the sense of e.g. not supporting tilde expansion, if not available). The benefit here would be, that we could keep code out of fzf which makes maintenance simpler.
  But you already said you don't like this.
• We could provide a convenience script, that contains a copy of all the bash-completion functions that we use (like _fzf_tilde() or - if you accept my other PR with the SSH host stuff - _known_hosts_real(). Around it a sentinel that prevents definition of the functions if already defined (by bash-completion). From time to time fzf could just update that with the current code from bash-completion.
  That would allow people who don't want to install bash-completion, to still use all functionality, and "us" to still "depend" on bash-completion, respectively outsource functionality to that. It would also allow us to get rid of fallback code like in my PR:
  

  fzf/shell/completion.bash

  Lines 297 to 306 in 8b5074d

  	else
  	# otherwise, get a list of hosts on our own
  	__fzf_list_hosts() {
  	command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
  	<(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts 2> /dev/null | tr ',' '\n' | tr -d '[' | awk '{ print $1 " " $1 }') \
  	<(command grep -v '^\s*\(#\|$\)' /etc/hosts 2> /dev/null | command grep -Fv '0.0.0.0') |
  	awk '{if (length($2) > 0) {print $2}}' | sort -u
  	}
  	fi

  
  Either the _known_hosts_real() would be there from bash-completion itself, or, as a copy, from our convenience script.
• Another, probably controversial ^^, idea would be to integrated the whole fzf-completion into bash-completion itself, as kind of an optional add on, in the sense of: if fzf if is available, bash-completion uses it.
  I spend quite some amount of thinking recently, how the most generic system of fzf-completion could be done... integrating it into bash-completion would be one flavour of it. I'll write some text the next days where I collect my ideas and ask for comments form people.
  At least this would allow fzf to be just fzf itself.
  That way would of course be even "worse" like a "soft" dependency on bash-completion.

We should at least expand $ENV_VAR, ~, and ~other_user_name. I can imagine many users would do something like vim $PROJECT1_HOME/**.

If you have a good idea t do the $VAR expansion, tell me. :-D

I first thought we could do some sed magic to get that, but as said before, it might by e.g. '${HOME}'/**<Tab> or perhaps "$( printf '%s-suffix' "${HOME}" )"/**<Tab> and I'd say we'd require a shell parser to determine that in these cases the var shouldn’t be expanded (even if we leave the rest unexpanded).

Actually, expanding vars may even destroy some uses cases:

If we expand the var in the command, it will get written to the history like that - so a user cannot just go back in the history and execute the same command, with the var (which might however have a changed value meanwhile).

Sure, for $HOME a change is unlikely, but for most others, not.

[calestyo]

Of course, the last point also applies to my idea of expanding shell patterns (*,?, etc.) ... again, the expanded command would get into the history which is likely not what's desired.

Maybe, readline’s shell-expand-line is what people should use in such cases instead.

[junegunn]

A simple fix: ee4ba10

Fuzzy completion is not provided if the prefix contains a pattern that is potentially dangerous. I think this will cover 99.9% of the use cases (all existing test cases are passing) and remove the risk. Please let me know if there are any other risky scenarios. ssh -i $(echo PWND > /tmp/FOO)**<TAB> will no longer work, but the default completion (without **) doesn't work either in this case anyway, so I don't think it matters.

Actually, expanding vars may even destroy some uses cases

Good point. But it can be tricky to get it right. This example shows an approach that doesn't expand the prefix, the problem is the preview program will no longer work.

TEMP=/tmp
dir='$TEMP'
eval "dir_eval=$dir"
if [ -d "$dir_eval" ]; then
  (cd "$dir_eval"; ls) | sed "s@^@${dir}/@" | fzf --preview 'cat {}'
fi

cat: $TEMP/...: No such file or directory

[calestyo]

A simple fix: ee4ba10

At first I though I could easily find a way to still break that, but turns out it looks mostly good (at least for a start).

1. I didn't manage to get undesired side-effects (like via $(…)) to be executed though don't understand why your checks are enough.
   Why don't you need to check for the other control operators?

      control operator
            A token that performs a control function.  It is one of the following symbols:
            || & && ; ;; ;& ;;& ( ) | |& <newline>
   

   Is it because COMP_WORDS will always be split at these? But is that even truly and always the case? What if IFS or COMP_WORDBREAKS was changed by the user? I tried the latter (removed ;), but as soon as I do so, no completion works anymore at all.
   The default COMP_WORDBREAKS is $' \t\n"\'><=;|&(:' (dollare-single-quotes quoted string) .. so that might already kill any of the above control operators, if present. Also {… ;} and other reserved words like if, etc. only work with ; or \n, which are also in the default COMP_WORDS.
   

   Tried quite hard, with commands like ssh -i $' 'if$'\n'true$'\n'then$'\n'myfunc$'\n'fi**, but since we eval only once, no problem.

   Still, me not finding more than what's listed below, is still no real proof, that it's actually save (beyond the stuff below). So while the feature is actually nice, my stomach says it feels fragile

However… 😈

2. I think the ${parameter:=word} constitutes an undesired side-effect. (At least I found something, my honour😝 is saved - I just wasted some hours for this)
   Take e.g.:

   $ unset -v FOO
   $ ssh -i ${FOO:=bar}**<TAB+abort fzf+no Enter>
   $ echo $FOO
   bar
   

   May I suggest to expand your checks to either exclude any "extended" forms of parameter expansion or allow just those for which we know they're save?
   I mean this may not sound like the biggest threat... but often, some smart guy even finds a way to use such things for attacks.
   Interestingly, = is part of COMP_WORDBREAKS, so no idea why it's even one comp word - guess I misunderstand something there.

3. I'm not sure why you include [[ $cur != *'<('* ]], cause for me, when I do e.g. ssh -i date<(myfunc)** completion doesn't even start. But if you think it's needed (and better have it rather than not), shouldn't we also include >(? For the other form of process substitution?

---

In general, I'd feel much safer if your check was much broader, e.g. something like:

`… && [[ $cur != *[$' \t\n"\'><=;|&(:']* ]]`

which I'd probably extend by ) (see (4) below).
that would also save us any possible troubles one could fiddle around with COMP_WORDBREAKS.

---

4. Completing a readline like ssh -i \$\(HOME)**<TAB> gives a syntax error: -bash: syntax error near unexpected token ')'

5. As does ssh -i '$(HOME}'**<TAB> and ssh -i '$(HOME)'**: -bash: unexpected EOF while looking for matching '''
   This happens here:
   

   fzf/shell/completion.bash

   Line 145 in e833823

   $orig "$@"

   
   Though I don't understand why.

6. Last but not least, we obviously have many false positives:
   ssh -i $HOME'$(notReallyEvilCmd)'**, ssh -i $HOME\$(notReallyEvilCmd)**, etc. should in principle all be save to be expanded (of course only $HOME would be - the command substitution is quoted).

---

Other questions:
a. Why does _fzf_complete() perform no eval wouldn't it also make sense to parameter expansion there? E.g. ssh $REMOTE_HOSTNAME or so.

b. Cosmetic:

fzf/shell/completion.bash

Line 173 in e833823

if [[ "$cur" == *"$trigger" ]] && [[ $cur != *'$('* ]] && [[ $cur != *'<('* ]] && [[ $cur != *'`'* ]]; then

fzf/shell/completion.bash

Line 238 in e833823

if [[ "$cur" == *"$trigger" ]] && [[ $cur != *'$('* ]] && [[ $cur != *'<('* ]] && [[ $cur != *'`'* ]]; then

You use now two different quoting styles (with and without "…") shouldn't matter though for [[ … ]], IIRC bash doesn't perform word splitting there, does it? Still, I personally like to quote it.

[calestyo]

btw:

Actually, expanding vars may even destroy some uses cases:
If we expand the var in the command, it will get written to the history like that - so a user cannot just go back in the history and execute the same command, with the var (which might however have a changed value meanwhile).

Good point. …

I'd no longer consider my own argument valid. ^^

I mean if someone does **<TAB>, then we can assume that he actually wants fzf-completion. That however, would never make sense, as far as I can think, if the parameter expansions are kept unexpanded. So from that PoV (and beside all the concerns about it being fragile) - expand it.

If however, if a user doesn't want expansion - well than simply don't use fzf-completion on that word.

And I mean, if one does e.g. ssh -o $FOO -i **<TAB>, then the $FOO in the other word, won't get expanded.

[calestyo]

Actually, there may be an exception from the above:

IIRC, a while ago, either bash or bash-completion, caused something like $ ls foo*<Tab> to be expanded to one of the matching files - which was quite annoying. Every time I accidentally did <Tab> I lost my nice pattern.

I for example, made a keybinding that does **<Tab> on <Shift-Tab>,... and hope to be able to improve it from the poor-man's solution it is now.

So assuming I'd accidentally press ... I might still want a way to not do the eval, even if it's just parameter expansions. OTOH, I could still abort fzf and would keep the unexpanded readline (it seems).

Well not sure what make sense... maybe it would be nice to have a config option that allows to disable the eval... maybe it's overkill.

[calestyo]

Oh and have you had a chance to double check the other evals?

Especially this little fella:

fzf/shell/completion.bash

Line 358 in a3ff49a

eval "$def"

[junegunn]

Wow, you tried really hard to break it, thank you. It's a relief to hear that you couldn't find a way to run an arbitrary command.

I can't answer all your questions right now because my time is limited and I don't have a comprehensive knowledge of the inner workings of bash. I'm looking for practical solutions through a series of trial and error, and I'll move on when I feel they're good enough, and then the community will fill in the missing pieces.

4. I'm not sure why you include [[ $cur != *'<('* ]], cause for me, when I do e.g. ssh -i date<(myfunc)** completion doesn't even start. But if you think it's needed (and better have it rather than not), shouldn't we also include >(? For the other form of process substitution?

I noticed that too, but that part is required for zsh and I wanted to put the same code in two implementations so I added it anyway. Wouldn't hurt. Not including >(...) was an oversight (I rarely use that form of process substitution).

I also found that ls <(ls -al)**<tab> spits out an error. $cur is -al) in that case.

In general, I'd feel much safer if your check was much broader, e.g. something like:

Agreed. I think we'll probably be fine once we reject a few more characters.

[junegunn]

What perplexes me is that when you trigger fuzzy completion on vim ${HOME:-foo}**, ${COMP_WORDS[COMP_CWORD]} is ${HOME:-foo}** which is expected, but when I select an item, filling in COMPREPLY, the prompt becomes vim ${HOME:/Selected/Path . Weird stuff.

The default completion doesn't trigger in this scenario, so we probably shouldn't try either. Do you know when the default completion is triggered or not? FWIW, it works on vim ${HOME}/<tab>. Maybe it's determined by the presence of COMP_WORDBREAKS characters?

[calestyo]

It's a relief to hear that you couldn't find a way to run an arbitrary command.

Well, point (2) above is in principle a side-effect which also allows for execution, consider e.g. ssh -i ${PROMPT_COMMAND:=date}**<TAB+abort fzf+no Enter> ... at least when PROMPT_COMMAND is unset. But there may of course be similar ways.

I can't answer all your questions right now because my time is limited and I don't have a comprehensive knowledge of the inner workings of bash.

I wrote another mail to help-bash, asking whether there's a real proper way for what we want to do.

I guess there won't be, but better ask.

I noticed that too, but that part is required for zsh and I wanted to put the same code in two implementations so I added it anyway. Wouldn't hurt.

Well it might lead to more false positives (probably not, if bash splits words on < or > ... but I don't really understand what it actually does with COMP_WORDBREAKS.

But apart from that, and what's worse IMO,... in half a year no one will remember why it was needed (or not) and this will make understanding the code more difficult in the future.

Not sure whether it's worth trying to keep the feature set for the shells so close.

Not including >(...) was an oversight (I rarely use that form of process substitution).

Are you going to add it, or shall I make a PR?

I also found that ls <(ls -al)**<tab> spits out an error. $cur is -al) in that case.

I think this is the same as my point (4) above. Again I don't understand why bash splits the command as it does, but the problem is the lone ) (and of course it would also be a, even worse, problem if the ( would be included).

Agreed. I think we'll probably be fine once we reject a few more characters.

My I propose first, that I split up the points from above into separate issues. I guess otherwise it gets to messy here. Ok for you?

[calestyo]

What perplexes me is that when you trigger fuzzy completion on vim ${HOME:-foo}, ${COMP_WORDS[COMP_CWORD]} is ${HOME:-foo} which is expected, but when I select an item, filling in COMPREPLY, the prompt becomes vim ${HOME:/Selected/Path . Weird stuff.

Indeed... Sounds like this could be some bug in bash? Would you mind to report it?

Or maybe it has to do with : being in COMP_WORDBREAKS?

The default completion doesn't trigger in this scenario, so we probably shouldn't try either. Do you know when the default completion is triggered or not? FWIW, it works on vim ${HOME}/. Maybe it's determined by the presence of COMP_WORDBREAKS characters?

No idea, sorry...

[junegunn]

when PROMPT_COMMAND is unset

Good point. Thanks for the enlightenment.

Not sure whether it's worth trying to keep the feature set for the shells so close.

Got it.

Errors on eval

I think it's safe to just suppress the error message. Or, we can just give up trying in those wacky cases.

Weird stuff

Removing : from COMP_BREAKWORDS fixes the issue, but that should be up to each user.

COMP_WORDBREAKS=${COMP_WORDBREAKS//:}

[junegunn]

6. Last but not least, we obviously have many false positives:
ssh -i $HOME'$(notReallyEvilCmd)'**, ssh -i $HOME\$(notReallyEvilCmd)**, etc. should in principle all be save to be expanded (of course only $HOME would be - the command substitution is quoted).

Can never know if a command is evil or not. Any side effects shouldn't be allowed. The normal tab completion doesn't trigger in those cases anyway, so yeah.

[calestyo]

488a236

Looks good on a first glance.

I think it's safe to just suppress the error message. Or, we can just give up trying in those wacky cases.

Might be enough. At least right now I cannot think of any way, where the syntax error could be abused or cause accidental damage (after all, it shouldn't even execute).

Removing : from COMP_BREAKWORDS fixes the issue, but that should be up to each user.

Hmm it still feels like a bug, which is only worked around by the removal of :.
If you're not willing to report it to bash-bug, I could, though I think my name should up a bit too often recently on help-bash, thus I think it might be better if you report it.

[calestyo]

Can never know if a command is evil or not. Any side effects shouldn't be allowed. The normal tab completion doesn't trigger in those cases anyway, so yeah.

Well sure, it's of course better to catch too much, than to few... but ideally a definite solution would just catch all the right cases.

[junegunn]

Well, I don't think I understand enough of the situation to report the bug (or feature?) in a concise way. A quick Google search revealed that people have been suffering from colon problems in completion for ages. Sigh.

• https://salsa.debian.org/debian/bash-completion/-/blob/master/bash_completion#L499-526
• https://stackoverflow.com/questions/65233239/compgen-doesnt-complete-words-containing-colons-correctly
• https://stackoverflow.com/questions/25362968/bash-command-line-tab-completion-colon-character
• https://gist.github.com/stecman/6cc2769b135f272b673a

[calestyo]

I've just noted some interesting things (Debian sid, with bash-completion 2.11 and bash 5.2.15):

$ mkdir -p 1/foo
$ v=1
$ ls $v/<Tab>

the <Tab> completes the last line to with no expansions:

$ ls $v/foo/

I mean that's IMO the best one can get: completions but not expansions

On the other hand:

$ ls $(echo 1)/<Tab>

is not completed

Further, after:

$ mkdir -p '$v/bar' '$(date)/baz'

we have:

$ tree
.
├── $(date)
│   └── baz
├── $v
│   └── bar
└── 1
    └── foo

Now:

$ ls '$v'/<Tab>

results in:

$ ls '$v'/bash: unexpected EOF while looking for matching `''
$ ls \$v/bar/

However:

ls \$v/<Tab>

correctly gives:

$ ls \$v/bar/

(that is: completion but no expansion).

Similarly:

ls '$(date)'/<Tab>

gives:

ls '$(date)'/bash: unexpected EOF while looking for matching `''
\$\(date\)/baz/

but:

$ ls \$\(date\)/<TAb>

expands correctly to:

$ ls \$\(date\)/baz/

No fzf functions were loaded here,... so somehow, bash-completion (?) seems to already do (mostly) what we'd also want.

[calestyo]

However…

$ mkdir -p '$(date)/1/awesome' '$(date)/$v/wonderful'
$ v=1
$ tree
.
└── $(date)
    ├── $v
    │   └── wonderful
    └── 1
        └── awesome

Now:

$ ls \$\(date\)/$v/<Tab>

completes to:

$ ls \$\(date\)/\$v/wonderful/

=> dammit... that should be awesome

and:

$ ls $\(date\)/'$v'/<Tab>

to:

$ ls \$\(date\)/\$v/wonderful/

=> that would be good.

[calestyo]

@junegunn Well... stupid me always looked at my Debian's bash-completion code (which is version 2.11, which is the most recent one - yet still hopelessly outdated) ;-)

It seems that bash-completion in git may already have what we need:
https://github.com/scop/bash-completion/blob/15b74b1050333f425877a7cbd99af2998b95c476/bash_completion#L159-L209

I only made some very simple checks so far, but it seems it even catches case (2) (i.e. ${parameter:=word}), well actually it simply might not allow any of the "extended" forms of parameter expansion.

[calestyo]

Made some more tests with _comp_dequote().

First some caveats:
https://github.com/scop/bash-completion/blob/15b74b1050333f425877a7cbd99af2998b95c476/bash_completion#L184-L202

The former (i.e. execution in cases like declare -n v='dummy[$(echo xxx >/dev/tty)]', might actually be a problem for us).

Not sure whether we could easily exclude that on our own... perhaps by searching for some smart pattern that allows only 0-9, * or @ after any [a-zA-Z_][a-zA-Z0-9_]*\[?

The case described there with $RANDOM... well TBH... if someone relies on such side effects, it's IMO on him to make sure that they're actually fulfilled.

Apart from that:
note that I changed output a bit: () denotes an empty ret array being printed, and I've added newlines between the checks for readability

$ foo="${HOME}"; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
/home/calestyo
0
/home/calestyo

$ foo='${HOME}'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
${HOME}
0
/home/calestyo

$ foo='\${HOME}'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
\${HOME}
0
${HOME}

$ foo='$\{HOME}'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
$\{HOME}
1
()

$ foo="\${HOME}"; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
${HOME}
0
/home/calestyo

$ foo="'\${HOME}'"; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
'${HOME}'
0
${HOME}

$ foo="$(date)'"; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
Fri Oct  6 04:03:21 PM CEST 2023'
1
()

$ foo='$(date)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
$(date)
1
()

$ foo='\$(date)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
\$(date)
1
()

$ foo='\$\(date)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
\$\(date)
1
()

$ foo='\$\(date\)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
\$\(date\)
0
$(date)

$ foo='\$\(date\)$HOME'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
\$\(date\)$HOME
0
$(date)/home/calestyo

AFAICS, this is all expected and what we'd also want.
, it even catches the cases, where (likely) no undesired expansion but a syntax error would occur (like when the input string is \$\(date), where the ) would cause the syntax error. So in that case, fzf-completion doesn't make anyway and it doesn't matter that the function gives back an empty array (with exit status 1).

It also seems to catch cases like:

$ foo='date;$(date)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
date;$(date)
1
()

$ foo=$'date\n$(date)'; printf '%s\n' "$foo"; _comp_dequote "$foo" ; echo $?; printf '%s\n' ${ret[@]}
date
$(date)
1
()

Which should hopefully still prevent any shenanigans in case the user changed COMP_WORDBREAKS.