Determine best way to resolve double-escape in post editing #31

Closed
opened 3 years ago by leftypol · 3 comments
leftypol commented 3 years ago
Owner
See: https://github.com/towards-a-new-leftypol/leftypol_lainchan/issues/302
leftypol added the
bug
label 3 years ago
Collaborator

image

![image](/attachments/7655d3ef-e955-44da-a831-4e22ecd6450a)
397 KiB
Collaborator

I have determined with confidence that the vichan fix is safe and correct.

The problem reported to lainchan was that editing a post subject could lead to HTML injection. https://github.com/lainchan/lainchan/issues/94

This same issue was noted by vichan a month later. https://github.com/vichan-devel/vichan/pull/230

antedeguemon correctly noted that the body is already sanitized and the email is saved using htmlspecialchars(). However the lainchan patcher used escaping on all the fields, resulting in double-escaping.

Escaping has to be done in the PHP rather than the HTML if $config[html_minify] is enabled (such as on Leftypol) or else there's no way to preserve newlines in posts (or other special whitespace).

(An alternative solution, possibly better and more internally consistent, is sanitize the name and subject files before storing. However this solution is simple and has no side effects to deal with, and brings it inline with vichan's development)

I have determined with confidence that the vichan fix is safe and correct. The problem reported to lainchan was that editing a post subject could lead to HTML injection. https://github.com/lainchan/lainchan/issues/94 This same issue was noted by vichan a month later. https://github.com/vichan-devel/vichan/pull/230 antedeguemon correctly noted that the body is already sanitized and the email is saved using `htmlspecialchars()`. However the lainchan patcher used escaping on all the fields, resulting in double-escaping. Escaping has to be done in the PHP rather than the HTML if `$config[html_minify]` is enabled (such as on Leftypol) or else there's no way to preserve newlines in posts (or other special whitespace). (An alternative solution, possibly better and more internally consistent, is sanitize the name and subject files before storing. However this solution is simple and has no side effects to deal with, and brings it inline with vichan's development)
Collaborator

Closed by #54

Closed by https://git.leftypol.org/leftypol/leftypol/pulls/54
discomrade closed this issue 2 years ago
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.