Moodle
  1. Moodle
  2. MDL-26077

Messaging silently strips all text between < and > characters

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.3
    • Component/s: Messages
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      Steps to reproduce:

      1. go to moodle.org or any other Moodle 2.0 site with messaging enabled
      2. send a message like

      Please contact our support <support@moodle.com>

      Expected behaviour:
      The message should be delivered as is typed

      What happens:
      Silent data loss - only the following message arrives

      Please contact our support 

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Helen Foster added a comment -

            Assigning to our messaging expert, Andrew.

            Show
            Helen Foster added a comment - Assigning to our messaging expert, Andrew.
            Hide
            Andrew Davis added a comment - - edited

            Im not sure what to do about this. The full message stores the message in full so the version that is emailed out, for example, is intact. However messaging UI uses the "smallmessage" version which has had strip_tags() called on it when the message was sent between users. This is to stop users from injecting potentially harmful html.

            What this means in practice is that this

            this is my <p>message</p>
            

            will result in a message that says "this is my message". The p tags are removed but not the text within them.

            However in the case of this...

            Please contact our support <support@moodle.com>
            

            removal of what looks like a html tag results in part of the message being stripped out.

            Show
            Andrew Davis added a comment - - edited Im not sure what to do about this. The full message stores the message in full so the version that is emailed out, for example, is intact. However messaging UI uses the "smallmessage" version which has had strip_tags() called on it when the message was sent between users. This is to stop users from injecting potentially harmful html. What this means in practice is that this this is my <p>message</p> will result in a message that says "this is my message". The p tags are removed but not the text within them. However in the case of this... Please contact our support <support@moodle.com> removal of what looks like a html tag results in part of the message being stripped out.
            Hide
            Joshuah Alan Kuttenkuler added a comment -

            http://www.php.net/manual/en/function.htmlspecialchars.php
            strip_tags() should escape messages with htmlspecialchars() to preserve links
            but avoid delivering potentially harmful html scripts. This
            is one of THE oldest rules in the book.

            Show
            Joshuah Alan Kuttenkuler added a comment - http://www.php.net/manual/en/function.htmlspecialchars.php strip_tags() should escape messages with htmlspecialchars() to preserve links but avoid delivering potentially harmful html scripts. This is one of THE oldest rules in the book.
            Hide
            Andrew Davis added a comment - - edited

            After speaking to Martin about this both full and small message are now stored unsanitized and cleaned up with s() on the way out. s() calls htmlspecialchars() as part of its operation. Only checking data on output is the Moodle way to avoid data loss (like this bug).

            repo: git://github.com/andyjdavis/moodle.git
            branch: MDL-26077_message_strip_brackets2
            diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26077_message_strip_brackets2

            Show
            Andrew Davis added a comment - - edited After speaking to Martin about this both full and small message are now stored unsanitized and cleaned up with s() on the way out. s() calls htmlspecialchars() as part of its operation. Only checking data on output is the Moodle way to avoid data loss (like this bug). repo: git://github.com/andyjdavis/moodle.git branch: MDL-26077 _message_strip_brackets2 diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26077_message_strip_brackets2
            Hide
            Andrew Davis added a comment -

            PULL-513

            Show
            Andrew Davis added a comment - PULL-513
            Hide
            Helen Foster added a comment -

            Andrew, thanks for fixing this issue.

            Show
            Helen Foster added a comment - Andrew, thanks for fixing this issue.

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: