Fix htmlentities() is called on already converted data on import

[Maran23]

What does this PR aim to accomplish?:

The data exported by the teleporter is already converted by the htmlentities() method before. Therefore, we do not need to call it again during import, otherwise some characters will be displayed incorrectly.

Example to reproduce this:

1. Add an entry to the adlist with e.g. 'abc' as comment (don't forget the two single quotation marks)
2. Verify the following entry in the database (adlist) 'abc'
3. Now export the adlist table via the teleporter. The JSON entry will also look like this "'abc&#039"
4. Now import the adlist.json again
5. Verify the following entry in the database (adlist) 'abc'
6. This is then displayed like this in the Web-UI: 'abc' instead of 'abc', as the data got converted again on import via htmlentities()

How does this PR accomplish the above?:

Do not call htmlenties() on import as previously exported data is already converted with htmlenties(). You can check this also by checking the database.

> sqlite3 gravity.db "SELECT * FROM adlist"

What documentation changes (if any) are needed to support this PR?:

/

---

By submitting this pull request, I confirm the following:

1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
2. I have commented my proposed changes within the code and I have tested my changes.
3. I am willing to help maintain this change if there are issues with it later.
4. It is compatible with the EUPL 1.2 license
5. I have squashed any insignificant commits. (git rebase)

---

• I have read the above and my PR is ready for review. Check this box to confirm

[yubiuser]

Thanks for your PR. This is related to the this issue: #1928

---

Sidenote: As discussed over there, I still think

The whole encoding/decoding needs a proper re-write

as long as we have something hacky like this:

https://github.com/pi-hole/AdminLTE/blob/c2afe4221ac275a1c082e1d8e14ccbb6113b0e7b/scripts/pi-hole/js/utils.js#L11-L18

I see two ways to go:

1. Prevent adding "invalid" characters in the first place (e.g. '), don't do any conversion afterwards
2. Carefully re-write the whole escaping/unescaping of user/database input

[Maran23]

Thanks for your PR. This is related to the this issue: #1928

Ahh, I see, thanks for pointing that out.
Next time I should have taken a closer look at all the issues. 😄

The whole encoding/decoding needs a proper re-write

I agree. It is a bit hacky at this point. And I can see that this can be a security risk as well.
Not allowing special characters sounds more like fighting the symptom to me here.

[Maran23]

One idea that came to my mind is a CheckBox, labeled with something like Sanitize Inputs (default checked), which one can uncheck in order to import without the usage of htmlentities(). Probably with a ? button, tooltip or a note explaining the possible security risk when unchecking it.

[yubiuser]

I wouldn't want to add a new checkbox. Even with a note, users won't understand the implications. I tested your code and it's working as expected. We only need to decide if we see a risk, if the input is not sanitized.

[DL6ER]

I'd rather like if we would keep the sanitation there. We should, instead, de-sanitatize when exporting. I know that this will not solve the issue for existing exports, however, I'm also not aware of anyone having been affected by this before. There just seems to be a lack of importance IMO to (possibly lightheaded) drop this here. I do see an attack vector with users importing specifically crafted teleporter archives because someone of Reddit told them that doing so will solve their problem XYZ. We know there will be folks just running stuff off the Internet they don't understand and simply "see" what happens.

[Maran23]

see an attack vector with users importing specifically crafted teleporter archives because someone of Reddit told them that doing so will solve their problem XYZ. We know there will be folks just running stuff off the Internet they don't understand and simply "see" what happens.

Jep, I agree. That was also my concern when I read #1928 and all linked comments.
So yes, the best way to go would be to de-sanitize the settings on export.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[PromoFaux]

No longer relevant due to the removal of php from the development branch in preparation for v6