Attached is a patch that should resolve the security issue reported in SA-CONTRIB-2011-025 - Juitter - Cross Site Scripting (XSS)

CommentFileSizeAuthor
#15 juitter_security-1202870-15.patch739 bytesartofeclipse
#8 juitter_xss_fix.patch1.16 KBartofeclipse
#4 juitter_xss_fix.inc_.patch973 bytesartofeclipse
juitter-security-fix.patch1.48 KBcyborg_572

Comments

mohammed j. razem’s picture

mohammed j. razem
Location Santa Clara, CA
commented
Version: » 6.x-1.x-dev
Priority: Major » Critical

I've granted you access to maintain the project. Can you please commit to master and dev snapshot?
I've noticed you diffed against master branch.

Thanks.

cyborg_572’s picture

cyborg_572 commented

Done and Done.

I think so, anyway. I'm still getting the hang of git.

mauritsl’s picture

mauritsl commented

I found more XSS vulnerabilities in this module, which were not fixed by this patch.

artofeclipse’s picture

artofeclipse commented
StatusFileSize
new juitter_xss_fix.inc_.patch973 bytes

the attached is a patch that fix the XSS issue in SA-CONTRIB-2011-025 - Juitter - Cross Site Scripting (XSS)

artofeclipse’s picture

artofeclipse commented
Assigned: cyborg_572 » Unassigned
nilsja’s picture

nilsja commented

is it fixed? where can i download it? otherwise i would like to download the broken one. i am not affected by the xss problems due to user rights...

thanks,

nils

bertboerland’s picture

bertboerland commented

the project is still abandoned. If the new maintainer doesnt want to maintain and fix the module, Dutch Open Projects found the security xss vulnability (and to be honest a couple of others in the module as well) and is more then eager to take over or co maintain this module since we use this actively for our customers. many parts need a major rewrite and we are willing and able to maintain this project.

So cyborg_572 if you still want to maintain this module, please do. But if you cant or wont, please keep in mind that we will be able to do so or co-maintain it.

artofeclipse’s picture

artofeclipse commented
StatusFileSize
new juitter_xss_fix.patch1.16 KB

here is another patch to fix the XSS issue in this module, SA-CONTRIB-2011-025 - Juitter - Cross Site Scripting (XSS)

cyborg_572’s picture

cyborg_572 commented

As far as I understand it/can tell, I've only really got commit access, nothing else. As it is, my plans were only ever to maintain this until someone with more time and interest could be found to take over. The team I work for needed this module up and running for a clients site, so I was tasked with making that happen. As more security issues have cropped up, and as that project moves farther into the past, my availability for this is severely cut back. As it is I've been trying to find time to respond to this for the last week or so.

At the time it seemed to be a toss-up between maintain the module, or lose a valuable tool. Things have changed since then, so by all means, please do not let me get in the way of someone taking this module over. Let me know if I can do anything to help the process. This week has less going on than usual (I think) so I'll see if I can manage some basic maintaining while things get sorted out.

As a heads up to anyone wanting to take over: in the last project some real shortcomings in the Juitter library itself started to become apparent (mostly the odd incorrect URL). Either we're using it wrong, or it's starting to show it's age in relation to the twitter API. Just something to consider.

artofeclipse’s picture

artofeclipse commented

Hello cyborg_572,

I was trying to take over maintainership for this project, I think you don't have the access for commit or asign a maintainer for this module, right ?

if you still have the commit just apply the last patch.

Thanks,

Ma'moun.othman

maxmendez’s picture

maxmendez commented

I like to work in this module

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

It's great to see the progress here.


+			$output .= "<a href=\"{$link['href']}\" id=\"{$link['attributes']['id']}\" class=\"{$class}\"> " . check_plain($link['title']) . "</a>";

That should ideally be swapped out for the l() notation instead of hard-coding the a tag.

@artofeclipse - I'm glad to see your progress on a second patch.

@bertboerland @mauritsl - can you clarify where the vulnerabilities were that you found? At this point its fine to disclose them in public since the module is marked unsupported.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I should add: I can confirm that the patch applied already (i.e. this one) fixes the issue that the security team was aware of.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

On further review, the code in #8 is not ideal as the standard in Drupal is to filter on output: http://drupal.org/node/263002

@bertboerland - now that the security issue has been out for a while it's fine to mention all the issues you found in the public. It would be sad to make someone the new maintainer and then have them create a release and find out that it's not *all* fixed.

artofeclipse’s picture

artofeclipse commented
StatusFileSize
new juitter_security-1202870-15.patch739 bytes

Thanks greggles for taking time to check this issue again, I attached the a new patch that fix all the issue regarding the XSS vulnerability in this module.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 6.x-1.x-dev »
Issue summary: View changes
Status: Needs review » Closed (outdated)

I am closing this issue, as it has been created for a release that is now not supported.