DietPi-Globals | G_CONFIG_INJECT: Special chars need to be escaped function internally, in case added via variable

Created on 3 Nov 2018  ยท  16Comments  ยท  Source: MichaIng/DietPi

Information:

  • Software title | Transmission
  • Was the software title installed freshly or updated/migrated? Fresh install
  • Can this issue be replicated on a fresh installation of DietPi? Yes

Steps to reproduce:

  1. Create any password with an & (ampersand) symbol.
  2. transmission-daemon completely fails to load on boot, and webui is inaccessible.

Extra details:

Manually editing /transmission-daemon/settings.json and updating the password field allows for normal operation. (EX: "rpc-password": "password&123",)

Bug Information Solution available

Most helpful comment

Commit merged: https://github.com/Fourdee/DietPi/commit/471d0b5295133ea66c660403538278987b179159

  • Testing ๐Ÿˆฏ๏ธ
  • Issue can occur, when using abc\/abc within $1 or $4. However this is theoretically covered by error prompt that does not list forward slash as char that needs to be escaped. The backward slash needs to be escaped, if wanted literally, abc\\/abc works, the forward slash does not, abc/abc works as well.
  • However, inform user explicitly that forward slash does not need to/should not be escaped, since this will be done internally, since some might be used to this being required in combination with sed: https://github.com/Fourdee/DietPi/commit/beff8ec96efa2d3d19ce568f258a69ea1d5cb3f7
  • Benefit is definitely, that in case of file paths, there is not need to escape that mich characters, which always looks ugly and makes code worse readable ๐Ÿ˜‰.

Testing passed here, will mark as closed.

All 16 comments

@MistahDarcy
Thanks for this info. Hmm I guess there are other special characters unsupported then. Hmm not sure how to handle this best.

  • Replace/Remove magic characters from GLOBAL_PW, before applying to Transmission and inform user about it? But what about DietPi-Automation installs?
  • Only allow non-magic characters for the global pass? No good solution. It is an issue that there are still so much services that do not support special chars and they should be pushed to do so instead the other way round! No modern solution to only support alphanumerical chars...

Do you have a suggestion or idea how to handle this best, keeping modern security standards, allowing uninteractive install but keeping user from running into an unexpected situation? ๐Ÿค”

Notes:

[  OK  ] DietPi-Software | Setting in /etc/transmission-daemon/settings.json adjusted: "rpc-password": "password&123",
"rpc-password": "password    "rpc-password": "{a9a67b86744c2069f05eb2857f8c9e834ba04d6b.LGo72pW",123",

Re-run, G_CONFIG_INJECT:

"rpc-password": "password"rpc-password": "password    "rpc-password": "{a9a67b86744c2069f05eb2857f8c9e834ba04d6b.LGo72pW",123",123",

Same

G_CONFIG_INJECT '"rpc-password"' "\"rpc-password\": \"$GLOBAL_PW\"," /etc/transmission-daemon/settings.json

๐Ÿˆฏ๏ธ We need to check GLOBAL_PW and escape \ in G_CONFIG_INJECT function

GLOBAL_PW='password\&123'
G_CONFIG_INJECT '"rpc-password"' "\"rpc-password\": \"$GLOBAL_PW\"," /etc/transmission-daemon/settings.json

@MichaIng

I'am not comfortable/understanding working on your G_CONFIG_INJECT code yet, don't want to break it with my ignorance ๐Ÿคฃ ๐Ÿ‘

However, it looks like we need to check and escape & during the function at some point, else, the & leaks.

@Fourdee
Ah okay, so not an issue of Transmission.

Hmm that's bad, sed -Ei "0,\|^[[:blank:]]*$pattern.*$|s||$setting|" $file expands & even when added via variable.

I anyway planned to do all required escaping within the script. Will work on this for v6.18

PR up for G_CONFIG_INJECT internal escaping in target setting arg $2: https://github.com/Fourdee/DietPi/pull/2227

  • & added
  • Extended regex support removed, as this would have made everything more complicated and benefit is limited. Need to check all function calls for ext regex use, before it can be merged. โ‚ฌ: Ext regex required, otherwise \+ and \(a\|b\) will be taken as regex match instead of literally.
  • But would be great to test the function bullet prove. sed/grep syntax errors should not be possible any more now, if I haven't forgotten a special char. Debug/error code could then be removed.
  • NB: Within $1 and $4, ext regex match is possible, thus manual escaping of literal meant chars required, whereas $2 now takes everything literal, thus no escaping required.

Test:

  • dietpi
  • password&123

๐Ÿˆฏ๏ธ Nextcloud + Transmission.

@MichaIng
You legend ๐Ÿ‘ Great fix!

@MichaIng

Thinking of a global for auto escaping a string, eg:

root@DietPi:~# G_ESCAPE_EXPANSION '\$%&test`2'
\\$%\&test`2

' not supported.

Then we can use in dietpi-config > passwords etc?

local setting="$(G_ESCAPE_EXPANSION $2)"
[  OK  ] G_CONFIG_INJECT | Added setting \$%&test`2\ to end of file test
root@DietPi:~# cat test
# Added by DietPi:
\$%&test`2\

Then somehow add this to:

G_WHIP_RETURNED_VALUE
root@DietPi:~# G_WHIP_INPUTBOX 'test'
root@DietPi:~# echo $G_WHIP_RETURNED_VALUE
\\$%\&test`2\

Fails with password:

root@DietPi:~# G_WHIP_PASSWORD
\$%&test`2\
\$%\&test`2\

Due to whiptail?

@Fourdee
I thought so as well, but actually this is very dangerous, since commands handle special characters different. We can only auto escape, if we know exactly which command finally handles the string.

E.g. in G_CONFIG_INJECT it is only possible, since sed and grep use the same ext regex rules (at least thoughout all my testing). For this reason I also needed to use sed to add setting to end of file, since echo ... >> leads to different results, and I also need to pass output through sed, before giving it to G_DIETPI-NOTIFY for the same reason.

whiptail again handles special characters different than echo -e:

  • In whiptail everything is taken literally, besides \n, which is expanded as newline in every case, even \\n leads to a newline after a backslash, so not escapable.
  • All other special char expansion is done within the quoted string, before given to whiptail: $var and escaping dollar/baskslash.
  • In echo -e on the other hand follows it's many escape rules for terminal output format, color and stuff. So basically what is required, is escaping backslashes when handing a string to echo -e/G_DIETPI-NOTIFY, which is what I do via additional sed pipe in G_CONFIG_INJECT, but should be done via ${VAR//\\/\\\\}, when having the string already in a variable.
  • whiltail input values are completely taken literally according to my tests, so no escaping required when retrieving the variable. Only when passing it to some other command, echo or whatever, escaping according to it's rules is required.

Hmm, just verified error with G_WHIP_PASSWORD:

  1. entry: test\$test (backslash wanted literally)
  2. entry: test\\$test (๐Ÿˆด escaping of the backslash required, otherwise will not match!)
  3. Very strange, since both values are retrieved exactly the same way: https://github.com/Fourdee/DietPi/blob/dev/dietpi/func/dietpi-globals#L952-L953
  4. Then compared within [[ ]], so nothing happens to them in between, that should lead to something being expanded...
2018-11-11 17:36:09 root@micha:~# test='a\$a'
2018-11-11 17:36:22 root@micha:~# [[ $test == $test ]] && echo $test
2018-11-11 17:36:35 root@micha:~# [[ $test == "$test" ]] && echo $test
a\$a
  • When comparing, the second var call needs to be quoted... I somehow remember reading this somewhere, but though it only be required either within single braces or =~ queck... ๐Ÿค”.
    This is veeeeery bad, basically means we need to change this thouout all our code, when we comparing two variables: Always double quote the second one....
  • Seems to be a usability feature: Usually you put the string that you want to check as first argument and a matching pattern as second. WIth this you can even put your regex pattern into a variable:
2018-11-11 17:48:04 root@micha:~# test1='abc abc'
2018-11-11 17:48:41 root@micha:~# test2='abc[[:blank:]]abc'
2018-11-11 17:48:45 root@micha:~# [[ $test1 == $test2 ]] && echo match
match
  • A bid similar to G_CONFIG_INJECT now, where regex will be expanded in $1 and $4 as matching patterns, but not in $2 as setting to scrape/search. But at least there it is clear which argument should be which, in [[ ]] I always thought it does not matter where you put which.

Further testing:

2018-11-11 17:38:27 root@micha:~# [[ a\$a == a\$a ]] && echo $test
a\$a
2018-11-11 17:38:41 root@micha:~# [[ a\$a == 'a\$a' ]] && echo $test
2018-11-11 17:38:56 root@micha:~# [[ 'a\$a' == 'a\$a' ]] && echo $test
a\$a
  • Generally special chars are expanded, as expected, baskslash leads to following char taken literally, single quotes lead to everything taken literally. But as above, with variables it is done different on both sides...

@Fourdee
Solved: https://github.com/Fourdee/DietPi/commit/8b6108107b58758ef141e9054983bbd344db762c

But as said, we need to keep this in mind and solve thoughout all code!

G_ESCAPE_EXPANSION + G_CONFIG_INJECT test:
Password:

\`"$%&test

Strange, had to escape the ! in DietPiRocks!, but works fine in script.

root@DietPi:~# GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:DietPiRocks!)"
-bash: !: event not found


root@DietPi:~# GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:DietPiRocks\!)"
root@DietPi:~# echo $GLOBAL_PW
\`"$%&test

Same for dietpi pw:

root@DietPi:~# GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:DietPiRocks\!)"
root@DietPi:~# echo $GLOBAL_PW
dietpi

๐Ÿˆฏ๏ธ

#!/bin/bash
GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:DietPiRocks\!)"
echo $GLOBAL_PW

GLOBAL_PW='null'
GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:DietPiRocks!)"
echo $GLOBAL_PW

root@DietPi:~# ./test
dietpi
dietpi

๐Ÿˆฏ๏ธ

root@DietPi:~# GLOBAL_PW=0
root@DietPi:~# GLOBAL_PW="$(cat /var/lib/dietpi/dietpi-software/.GLOBAL_PW.bin | openssl enc -base64 -d -aes-256-cbc -nosalt -pass pass:'DietPiRocks!')"
root@DietPi:~# echo $GLOBAL_PW
dietpi

@Fourdee
As said, this is very dangerours: echo -e will do other expansion now, this will definitely lead to other issues! We need to do this individually, based on the exact path/commands the variables/strings will be passed to!

When you use this expansion now, which is tailored for grep and sed, before passing the variable to e.g. echo, cat, whiptail or somewhere else, you will have many backslahes there unexpanded, thus unexpacted wrong results! These expansions are grep/sed flavour of ext regex only!

@MichaIng

Yep, will remove from G_CONFIG_INJECT, but leave G_ESCAPE_EXPANSION in place for testing.

@Fourdee
Jep, perhaps we can find a use for this, when we have a couple of code where identical escaping is required, especially echo -e/G_DIETPI-NOTIFY I am thinking of, when we want to output certain variable strings literally. But AFAIK only backslashes need to be escaped for this, so a separate function is perhaps overkill, especially since a function cannot return a string, thus requires echo again, which requires again additional escaping properly ๐Ÿคฃ.

@MichaIng

Great work on this ๐Ÿ‘

I believe we can now mark this as completed. DietPi password supports special chars in v6.18.

I will reopen to remind myself about one last fix:

  • Extended regex use | within (option1|option2). With $4, the setting it is added via sed -i '0,\|$4|s||$2|' so | is used as delimiter, which breaks the use of ( | ). It is possible to escape | in $4, but not good to have this required. Better use some non-ext-regex character as sed delimiter and escape this as usual then in all three arguments.
  • Simply use default /?

Commit merged: https://github.com/Fourdee/DietPi/commit/471d0b5295133ea66c660403538278987b179159

  • Testing ๐Ÿˆฏ๏ธ
  • Issue can occur, when using abc\/abc within $1 or $4. However this is theoretically covered by error prompt that does not list forward slash as char that needs to be escaped. The backward slash needs to be escaped, if wanted literally, abc\\/abc works, the forward slash does not, abc/abc works as well.
  • However, inform user explicitly that forward slash does not need to/should not be escaped, since this will be done internally, since some might be used to this being required in combination with sed: https://github.com/Fourdee/DietPi/commit/beff8ec96efa2d3d19ce568f258a69ea1d5cb3f7
  • Benefit is definitely, that in case of file paths, there is not need to escape that mich characters, which always looks ugly and makes code worse readable ๐Ÿ˜‰.

Testing passed here, will mark as closed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

aesirteam picture aesirteam  ยท  3Comments

oshank picture oshank  ยท  3Comments

MichaIng picture MichaIng  ยท  3Comments

Fourdee picture Fourdee  ยท  3Comments

mok-liee picture mok-liee  ยท  3Comments