New notifications feature

[joedf]

Alright, exploit fix is working : Confirmed.
Test : http://ahkscript.org/boards/viewtopic.php?f=17&t=6065
Commit : https://github.com/ahkscript/phpBB_v3.0 ... e4701eb3a9

Exploit fix : Backslashes were not being escaped.
http://stackoverflow.com/a/3615890/883015

[jNizM]

Interdasting ^^

Everywhere Loading

[just me]

Everywhere Loading

Same here, that's why I asked whether it can be disabled by the user. One update when logging in is sufficient for me. I don't need any automatical updates afterwards.

[joedf]

Yes I had removed the minified js version to debug.
I'll put it back tonight

[joedf]

Ok, I have setup the minified version. Should be faster.

[lexikos]

See the error?

Code: Select all

var NotifsServerSide_JSON = '[{\"ti\":\"6089\",\"tlpt\":\"Jan 24 2015 07:52 PM\",\"tt\":\"[solved] "if var is upper" doesn't seem to work\",\"tlpn\":\"Benny-D\",\"tlpi\":\"35848\",\"tlpc\":\"\"},{\"ti\":\"6042\",\"tlpt\":\"Jan 24 2015 07:44 PM\",\"tt\":\"ImageList from HBITMAP\",\"tlpn\":\"cyruz\",\"tlpi\":\"35844\",\"tlpc\":\"\"}]';

' isn't escaped. Ironically, there are a whole lot of escaped double-quote marks that didn't need to be escaped.

[joedf]

Hmm thanks for reporting this
Hopefully this commit will fix it.
https://github.com/ahkscript/phpBB_v3.0 ... 9e502a9521

[lexikos]

StrLower / StrUpper / StrTitleCase? showed up in my notifications a bit weird. NotifsServerSide_JSON literal string contains "StrLower \\\/ StrUpper \\\/ StrTitleCase?" (escaped back slash followed by escaped forward slash). It shouldn't have any backslashes at all, let alone an escaped one - forward slash doesn't need to be escaped.

I'd guess you need to escape \ separately, before everything else, so that you don't escape the escape characters.

[joedf]

"StrLower / StrUpper / StrTitleCase? " showed up for me too.... weird..
commit: https://github.com/ahkscript/phpBB_v3.0 ... 7b6bfbfa73

[lexikos]

Code: Select all

+	$escapers = array("'","'");
+	$replacements = array("\'",'\"');

I suppose that your code (above) would replace ' with \' and leave " alone, because it's not in the $escapers array. If I'm reading the Examples of potential str_replace() gotchas correctly, it would then replace \' with \\". We don't want that.

Do you even need an array? If the JSON is only ever either returned as raw data or put inside 'single quotes', I suppose you only need to escape backslash and then single-quote. Is it possible for the data to contain a newline character or something else which would interfere with either JSON or JavaScript parsing? I see that you've removed the replacement of \n etc. I guess those characters are escaped when the JSON data is generated, before calling escapeJsonString.

I'm a bit confused by the JSON-mandated escape sequences in the JSON data then having to be escaped in the JavaScript string. Within the JSON data we have both " indicating the beginning/end of a JSON string and \" indicating a literal character in a JSON string. When this is transformed to JavaScript, it needs to become \\" to indicate the literal characters \ and ", which are later transformed to " when parsed as JSON. So I guess we don't want to escape the " on the JavaScript side, but escape the \. I think that's how it already works, assuming the literal characters in the JSON data have already been escaped appropriately before they are passed to escapeJsonString.

[joedf]

Whoops the 2nd "'" is supposed to be '"'.. Typo..
Yes the json_encode() is supposed to encode and escape everything but somehow it isn't...
And yes I had removed the \n since the topics can't have new lines.

fix typo : https://github.com/ahkscript/phpBB_v3.0 ... adb13bb491

[lexikos]

Isn't it completely unnecessary to escape "? I suppose either way is fine.

[joedf]

Isn't it completely unnecessary to escape "? I suppose either way is fine.

I thought so too, but apprently json_encode() isnt working correctly ...
Before this appeared "A_DoubleClick (HKCU\Control Panel\Mouse\DoubleClickSpeed)", no manual escaping was performed at all, since i thought that json_encode() was supposed to manage that... this topic proved me wrong... so it looks this manual escaping is needed...

[lexikos]

Did you maybe miss the " in my question? That topic name contains slashes, which I would expect should be escaped, and does not contain double quotes, which is what I was asking about.

Is the string which is passed to escapeJsonString a proper JSON string, like {"something":"\"quote marks\"", where literal quote marks are already escaped? I'd assume so, since I can't see it working any other way. If escapeJsonString was to replace all " with \"... well, it actually wouldn't have any effect at all if the string is just going to be put into var NotifsServerSide_JSON = '(string goes here)', because " and \" are the same in that context.

I think the topic name being written into JavaScript has to contain double escapes, so json_encode on its own wouldn't be enough. I think I'm repeating myself, so to put it briefly, JavaScript 'foo\\\\bar' -> JSON foo\\bar -> actual value foo\bar.

[joedf]

Ahh ok, I guess this is getting a little confusing
I'll do some testing tomorrow night. Thanks for the clarification.

[lexikos]

Did somebody do something with the notification feature? I got some (unwanted) email notifications earlier today, but "Email notifications" are still turned off.

[joedf]

What the? No.... Really weird!

[atnbueno]

An useful feature

But (just a minor annoyance) today I had 4 notifications and couldn't click on the fourth one because the "About ahkscript.org" forum was on top of it.


Regards,
Antonio

[joedf]

What browser are you using?

[atnbueno]

Last version of Firefox