HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Issue: Vanilla character encoding

mtschirsmtschirs ✭✭✭

All of the following can be found in the vanilla core:

htmlspecialchars(..., C('Garden.Charset', 'UTF-8'))
htmlspecialchars(..., 'ISO-8859-1'); // Smarty default
htmlspecialchars(..., 'UTF-8')
htmlspecialchars(...) // Defaults to 'ISO-8859-1' in PHP <= 5.3

From the PHP manual:

For the purposes of this function, the encodings ISO-8859-1 [and] UTF-8 [are] effectively equivalent, provided the string itself is valid for the encoding.

But what if C('Garden.Charset') is set to something more exotic? That is how you end up on http://seclists.org/ ...

Why even bother with C('Garden.Charset') then? Better make Vanilla UTF-8 only?

Comments

  • @mtschirs said:
    Why even bother with C('Garden.Charset') then? Better make Vanilla UTF-8 only?

    Vanilla is pretty much UTF-8 only. At least I wouldn't try using a different encoding.

    But really, this is the kind of issue to post on github.

    for reference, see this discussion:
    https://github.com/vanilla/vanilla/pull/2507

    But what if C('Garden.Charset') is set to something more exotic? That is how you end up on http://seclists.org/ ...

    Do you have an example for a working exploit?
    Also, it is not a user-facing configuration setting. If someone creates a vunerability by changing this without a reason it is their fault.

  • peregrineperegrine MVP
    edited August 2015

    I think this is an example why posting on forum in addition to github, is optimal. since it is easier to find here than on github. and it is read by a larger population of readers than github..

    @mtschirs very insightful discussions.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • mtschirsmtschirs ✭✭✭

    @Bleistivt: I just read the discussion + your pull request you referenced on github :+1:. Can you explain to me what the vanilla stage branch is for exactly? Shouldn't this be fixed in master?

  • BleistivtBleistivt Moderator
    edited August 2015

    See http://vanillaforums.org/discussion/27971/pull-requests-you

    Basically stage is where larger pull requests are tested before they are merged into master.

    ...At least it was used like that. I have been told that the stage branch is not used so much anymore.

  • another encoding related issue filed a 1 1/2 years ago.

    https://github.com/vanilla/vanilla/issues/1773

    not sure how it relates to current vanilla version

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • mtschirsmtschirs ✭✭✭

    From the current 2.2b1 announcement thread:

    @Linc schrieb:
    Here's some of what's new in 2.2:
    ...

    • Harmonize charset in htmlspecialchars.
      ...

    Did I miss something?

  • R_JR_J Ex-Fanboy Munich Admin
  • mtschirsmtschirs ✭✭✭
    edited August 2015

    @R_J: Yes, this was a good start, but why did they only convert a select few of all existing htmlspecialchars in the codebase? Now we have 3 competing standards instead of 2... isn't that the opposite of "harmonize"? :grin:

  • peregrineperegrine MVP
    edited August 2015

    edited.

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • LincLinc Detroit Admin

    The conversations on that PR gives a little background, I think. The best way to approach this would be to create a htmlEsc() function that wraps our repetitious implementation of htmlspecialchars() in a canonical way, and use that everywhere it's needed in Vanilla.

  • R_JR_J Ex-Fanboy Munich Admin

    Well, in this case "they" have been "me" and I either
    a) simply haven't seen the other files
    b) was only looking for a special pattern
    c) thought changing the other files is too much of a core change
    d) left the other work for some other people
    e) "I've harmonized all that uses any charset parameter"
    f) am only a noob who is already happy when I change only a selected few files and would have never commited any Pull Request if it had to be perfect.

    Honestly I do not remember.

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    @R_J I didn't want to come off as negative, I simply didn't make the connection between what Linc wrote about 2.2 <-> your pull request. It is a very good start indeed.

    @Linc: I am not convinced of "htmlEsc" yet, would need an "htmlUnesc" too. Also, "htmlspecialchars" is easy to spot and every PHP dev knows what it does. It would improve readability if we just stick with it and appropriately overwrite default params. This will be a non-issue in PHP >= 5.6 anyway due to the default_charset setting.

  • See https://github.com/vanilla/vanilla/issues/3047 - make Vanilla UTF-8 only

Sign In or Register to comment.