HTML tags are getting stripped

RBGE

rsclmumbai I must admit I noticed a similar (or probably identical) issue when using <. Somebody raised a ticket about purchasing consumables and my reply included "<£100"... everything after the < was stripped 😐️
I suppose a better option (anyone can feel free to correct me if there are issues here I haven't noticed 😇 ) would be to convert any < entered in the reply field into <... probably the same way this forum does! I'm too lazy to create a Github account just to post this though 😴

rsclmumbai

@RBGE Agree. Simply converting < into < & > into > takes care of security and content. Since OSTicket uses a ready library HTMLawed, its a default implementation I guess by the library.

rsclmumbai

A small issue. My solution above only works for HTML emails.
@ntozier Can you suggest what should be tweaked for TEXT emails?
Thanks a ton in advance!!

rsclmumbai

I scanned the entire class.format.php file but could not locate any function or code which works for text emails. Any pointers are appreciated.
Thanks

ramrajone

rsclmumbai
Take a look at this its little old but may help
https://github.com/osTicket/osTicket/issues/3479
change line 218 from ./scp/js/scp.js
from
redactor.insert.html(canned.response);
to
redactor.insert.html(canned.response, false);
false means no stripping...

https://github.com/osTicket/osTicket/commit/82c31641375424c69b6103a00719ea246c288f9a

rsclmumbai

Thanks @ramrajone
In the latest version of OSTicket, this code is on line 238.

redactor.insert.html(canned.response, false);

I modified the code as you recommended but it did not help. In my case though, the issue is not with canned responses but with incoming text/plain emails. This issue if really hitting me hard.
It would be really great if someone can suggest a code edit.

Just to recap, commenting the following line helped in fixing this issue with text/html emails.

includes / class.format.php
#$text = Format::safe_html($text, array('spec' => $spec));

But I'm unable to find the code which needs to be modified for text/plain emails.

Thanks in advance!

ramrajone

rsclmumbai

Take a look at this link, the reason HTML tags removed for incoming emails to help protect vulnerable to XSS:
https://github.com/osTicket/osTicket/issues/4511

https://github.com/osTicket/osTicket/issues/3791#issuecomment-500506685

rsclmumbai

Thanks @ramrajone but I did not really follow what you are trying to convey 🙁
I checked both the links you shared and I agree, HTMLawed is improving security. In my case, since its a specific use case, I need to show content between <>.

My mail server is hosted on Cpanel, I'm trying to see if I can make any changes to the incoming email on-the-fly and convert it from text/plain to text/html. Fortunately, I'm able to bypass the filters for html emails.

Thanks again

rsclmumbai

On another note, what changes can I make to completely bypass HTMLawed for all types of emails - text & html?

ramrajone

rsclmumbai
again not best security practice, but I think you can, I am not sure maybe DevTeam can confirm @KevinTheJedi

https://github.com/osTicket/osTicket/blob/develop/include/class.format.php#L338-L347
Line: 345
function sanitize($text, $striptags=false, $spec=false)

rsclmumbai

Already done that. My code looks like this:
` function sanitize($text, $striptags=false, $spec=false) {

    //balance and neutralize unsafe tags.
    //$text = Format::safe_html($text, array('spec' => $spec));
    //above line commented on 2020-02-05 to avoid stripping of <111111>
    $text = self::localizeInlineImages($text);

    //If requested - strip tags with decoding disabled.
    return $striptags?Format::striptags($text, false):$text;
}`

This is working fine but only for text/html emails.

If I get an email with content-type = text/plain, then the <xxx> is still stripping.

ramrajone

rsclmumbai
Do you have Enabled Rich Text?
/scp/settings.php

rsclmumbai

@ramrajone Yes.

KevinTheJedi

@rsclmumbai

Read my response here:

https://github.com/osTicket/osTicket/issues/5368

Cheers.

rsclmumbai

Thanks @KevinTheJedi -- it worked.

File: include/class.thread.php

inside the class:
class TextThreadEntryBody extends ThreadEntryBody {
I commented the following code:

function getClean() {
        return Format::htmlchars(Format::html_balance(Format::stripEmptyLines(parent::getClean())));
    }

Hope it helps others looking for a similar solution.

Happy weekend!!

KevinTheJedi

@rsclmumbai

I hope no one else implements this as it’s a security risk. This does not open you to XSS attacks only, people can use XSS to start different, more serious attacks or possibly execute a different attack altogether. Just all around not a good idea. I understand your use case and such but really not a good idea from security standpoint.

At some point in the future we hope to implement code blocks or something similar to say “Don’t sanitize/balance content inside this block of code”. This will alleviate most of these types of issues. This is not set in stone, just on our feature request list.

Cheers.

rsclmumbai

Thanks @KevinTheJedi
I totally agree with you. I'm able to take this risk since we are using OSTicket strictly internally.
Nevertheless, I hope to see the Don't Sanitize..... feature in the future releases.
Thanks for the awesome helpdesk system & superb support!!

KevinTheJedi

rsclmumbai

This is slated for v2.0 when we have better sanitization, etc.

Cheers.

ChrisAnders

@KevinTheJedi was there ever any progress on this? I know it was a couple of years ago now...

asmith3006

Is there anything we can do now on this? We use our osTicket for a tech support help desk so often emails contain snippets of HTML or XML which all get stripped out.

Surely encoding < as < and > as > would resolve this problem? then it would be displayed rather than rendered?

I appreciate this is coming in v2, but (correct me if I'm wrong?) we don't know when that's coming.

KevinTheJedi

asmith3006

You can modify the codebase but you’ll likely open yourself up to XSS and similar attacks. Proceed at your own risk.

Cheers.