Conversation
Notices
-
@moonman you know you can just restrict attachments allowed, right?
$config['attachments']['supported'] = 'image/png,image/gif, etc, etc';-
@maiyannah different problem.
-
@moonman What's up then?
-
@maiyannah add that header I just posted to your http config.
https://git.gnu.io/h2p/Qvitter/blob/master/js/dom-functions.js#L1287
replace the "unescape"'ed portion in the link above with a hardcoded string or something.
like, right now. -
@moonman Oh I am pretty sure I removed that a bit back because it could let people inject code into the javascript, unless it got added back in by a subsequent qvitter update.
Let me guess, someone did that? -
@mmn @moonman @maiyannah the thing is that gs itself sends html in the source field in the api http://qttr.at/1gns
-
@hannes2peer @moonman @mmn Is source set at any point by qvitter, such as composing notices?
-
@moonman @mmn @hannes2peer (Trying to think of how this exploit actually worked)
-
@maiyannah yes
-
@maiyannah the user (client) can send a any "source" when posting to api. imo it should be treated/sanitised by gs just like the notice text
-
@hannes2peer Agree, I thought it was put through HTMLPurifier like the rest, though?
-
@maiyannah apparently not
-
@hannes2peer Looking now, as far as I can tell /lib/apistatusesupdate.php just takes $this->source; without any filtering, unless it happens elsewhere in the code and I am missing it.
-
@maiyannah and now we'll have to assume it might have not, even if it's fixed in newer gnusocial
-
@hannes2peer cc @mmn
-
@hannes2peer (Which is entirely possible, I am not very familiar with the API side of things yet.)
-
@mmn @hannes2peer https://git.gnu.io/gnu/gnu-social/blob/master/actions/apistatusesupdate.php#L315
Only thing I see before then is a check to make sure it's not empty. -
@maiyannah this is what i did to !qvitter https://git.gnu.io/h2p/Qvitter/commit/632d5f113627df4c158be973aefc1afc018764f4
-
@hannes2peer For GS end, do you think cleaning it in the same way as a notice text gets, is enough?
-
@maiyannah yes htmlpurifier should be enough, i guess?
-
@hannes2peer I will make this change to my fork that some people use at least.
-
@hannes2peer @mmn I think they made the link go somewheres and that's how they managed it? @moonman could tell you, it was his instance that was affected directly, the others were just affected indirectly from federated posts.
-
@maiyannah @mmn e.g. i could do <script>alert("hello")</script> but not <script>console.log("hello")</script>
-
@mmn @hannes2peer ... @moonman, for some reason on my end that didn't link.
-
@maiyannah ok. but the source field is not federated.
-
@hannes2peer This is why I don't have a good idea really what the vector was. I was just a spectator trying to help out.
-
@hannes2peer Reasonably source shouldn't be html at all! So just escaping it on output is good enough I think.
-
@hannes2peer @maiyannah I've been looking at it now and I'm curious where someone can put their own URL in there in a way that will be output to !qvitter since it's only HTML if Notice->getSource returns a Notice_source object, which should be under server control and not affected by user input.
-
@mmn Notice_source is only for known sources. unknown sources are served directly from the notice table
-
@hannes2peer @maiyannah Ah no alright, I didn't read the whole getSource function: https://social.umeahackerspace.se/url/94331
That thing means API clients can choose their own source names (which is a good thing) and match against OAuth applications (matching up so the name gets linked).
It's when the HTML in !GNUsocial gets built that nasty stuff get in. I have now redacted this whole procedure and the URL is sent as source_link, separately from the name (which is now never HTML). -
@hannes2peer @maiyannah I'm sorry, I goofed. I thought somebody already did for some reason and I didn't want to flood your mentions.
-
@maiyannah @mmn @hannes2peer I have a copy of my db before I cleansed notices etc. It looked like it relied on the fact that html is very forgiving of malformed tags, and the injected domain was short.
-
@moonman @mmn @hannes2peer I cant speak for the other two but I'd be curious to see the payload and specific effect, you have my email and should have my PGP if you want to keep it secure.
-
@moonman @mmn @hannes2peer ... I always FrankerZ that acronym up.
-