Teleporter: use of apostrophes results with clients desc "'" and adlists desc "'"

[rharmonson]

Versions

• Pi-hole: Pi-hole version is v5.5 (Latest: v5.5)
• AdminLTE: AdminLTE version is v5.7 (Latest: v5.7)
• FTL: FTL version is v5.10.2 (Latest: v5.10.2)

Platform

• OS and version: Raspbian GNU/Linux 10 (buster)

• Platform: Raspberry Pi

Expected behavior

When using an apostrophe (') in Adlists or configured client description, the expectation is Teleporter will export and import original character.

Actual behavior / bug

The apostrophe is imported as "'" or "&#039"

Steps to reproduce

Steps to reproduce the behavior:

Source pi-hole

1. Login to the Pi-hole admin web interface
2. Select "Settings" from the navigation menu
3. Select "Teleporter" tab
4. Select the "Backup" button
5. Save backup-file

Target pi-hole

1. Login to the Pi-hole admin web interface
2. Select "Settings" from the navigation menu
3. Select "Teleporter" tab
4. Select the backup-file from source pi-hole
5. Select the "Restore" button
6. Select "Group Management" from the navigation menu
7. Select "Clients" from the navigation menu
8. Note all descriptions using an apostrophe on the source pi-hole are imported as "'"
9. Select "Adlists" from the navigation menu
10. Note all descriptions using an apostrophe on the source pi-hole are imported as "'"

Debug Token

• URL: https://tricorder.pi-hole.net/KEzhtKXq/

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

echo $LANG
en_US.UTF-8

[yubiuser]

The reason is that line

https://github.com/pi-hole/AdminLTE/blob/50f43bde73d25211ac2c81e9092be67512611d2a/scripts/pi-hole/php/teleporter.php#L199

converting all characters to their respective HTML entities. This prevents insertion of malicious code. The same issue was discussed here

#1733 (comment)

I still think the security aspect outweigh the visual bug. But happy to here from @pi-hole/web-approvers

[XhmikosR]

Just thinking out loud, can't we use html_entity_decode? Unsure about any implications, but it does the opposite of htmlentities IIRC.

[rharmonson]

I reviewed 1733 and appreciate the response.

My thoughts as a security professional.

Exporting and importing data must result with the same values unless changes are intentional, i.e. data integrity. If the user is entering unexpected values then prevent it through input validation. To improve the user experience, communicate on the form what characters are permitted or not permitted. Alternatively, alert them when input validation fails.

I am hazy on the security concern. I saw malicious code being cited but what is the threat vector? Where I am going with this is the threat and vector, generally, dictate the security control, so is the current remediation appropriate or even effective?

Not meaning to poke holes. Loving Pi-hole and the work you guys are doing.

[rharmonson]

In lieu of a bug fix as a long term solution, I was able to import my Teleporter export file after corrections using a shell script.

Hopefully, this issue will be resolved soon, but in the meantime I am able to move forward.

#!/bin/bash
# Usage: after exporting your data using Teleporter, copy the export and this shell script to a directory, then execute.
# Syntax: fixpht.sh FIND REPLACE FILE
# where FIND is the string to search, REPLACE is the replacement string, and FILE is the Teleporter export file
# Example: ./fixpht.sh "'" "'" test-teleporter_2021-10-17_11-34-55.tar.gz
find=$1
replace=$2
file=$3
mkdir pht.tmp
tar -xzvf $file -C pht.tmp
sed -i "s/$find/$replace/g" pht.tmp/*.json
mv $file backup-$file
cd pht.tmp
tar -czvf $file *
cp $file ..
cd ..
rm -fr pht.tmp

[yubiuser]

I had a look in the code again and the data ist already stored in the database in escaped form. So this is not an issue with Teleporter directly.

This was introduced by #1443. The cosmetic markup not displaying "'" was introduced here #1603

[rharmonson]

@yubiuser, nice catch!

Let me know if I need to update the title or take some action to push it forward. I appreciate your timely response and efforts.

[yubiuser]

Thanks, no need to change the title. We are aware of this bug. The whole encoding/decoding needs a proper re-write. We just need to find the time to do this.

some action to push it forward

PR's are always welcomed :)

[rharmonson]

Understood. Thank you.