Security fix for coppermine: EXIF XSS vulnerability *MUST READ*

[MerNion]

• users running cpg1.3.3 should download the file attached, rename it from "displayimage.txt" to "displayimage.php" and upload it to their webserver into the coppermine root folder, replacing the existing file on the server.
  

Some of us have heavilu modified the viewimage.php file to meet our needs. If we just make the changes you mentioned (find/replace), would that be ok to fix the problem?

[Joachim Müller]

viewimage.php: there's no such file in the coppermine distribution afaik, but if you're refering to displayimage.php: yes, it's safe to just do the suggested changes in the code - that's why we posted them.

[Makc666]

1. Difference between 1.3.3 and 1.3.4 is only that fix??

2. I checked displayimage.php from 1.3.3 archive donwloaded from this site and there is no such block of code as:

Code Select Expand

    if (isset($iptc) && is_array($iptc)) {
        if (isset($iptc['Title'])) $info[$lang_picinfo['iptcTitle']] = trim($iptc['Title']);
        if (isset($iptc['Copyright'])) $info[$lang_picinfo['iptcCopyright']] = trim($iptc['Copyright']);
        if (!empty($iptc['Keywords'])) $info[$lang_picinfo['iptcKeywords']] = trim(implode(" ",$iptc['Keywords']));
        if (isset($iptc['Category'])) $info[$lang_picinfo['iptcCategory']] = trim($iptc['Category']);
        if (!empty($iptc['SubCategories'])) $info[$lang_picinfo['iptcSubCategories']] = trim(implode(" ",$iptc['SubCategories']));
    }

There is block of code:

Code Select Expand

    if (isset($iptc) && is_array($iptc)) {
        if (isset($iptc['Title'])) $info[$lang_picinfo['iptcTitle']] = trim($iptc['Title']);
        if (isset($iptc['Copyright'])) $info[$lang_picinfo['iptcCopyright']] = trim($iptc['Copyright']);
        if (isset($iptc['Keywords'])) $info[$lang_picinfo['iptcKeywords']] = trim(implode(" ",$iptc['Keywords']));
        if (isset($iptc['Category'])) $info[$lang_picinfo['iptcCategory']] = trim($iptc['Category']);
        if (isset($iptc['SubCategories'])) $info[$lang_picinfo['iptcSubCategories']] = trim(implode(" ",$iptc['SubCategories']));
    }

I think that you made a mistake in your first post...

[Joachim Müller]

1. Difference between 1.3.3 and 1.3.4 is only that fix??

No, minor changes and fixes are made all the time in the cvs. When a new package gets released, those fixes go into the package as well. None of the other fixes are security-related, so I didn't post them. The security fix is not the only difference between cpg1.3.3 and cpg1.3.4

I think that you made a mistake in your first post...

I won't comment this, maybe the dev who took care of the fix wants to. In fact, the lines do the same, there is only a cosmetical issue.

Joachim

[Tarique Sani]

He He!
@Makc666 - I wouldn't bother about the change between the two code blocks

[ramppi]

Some error, when replacing those two 'pices' of code (first post)

Parse error: parse error, unexpected T_STRING in /home/XXXXXX/public_html/galleria/displayimage.php on line 310

310 is that Aditya-line

regards
Matti

[Aditya Mooley]

You must have missed a / from that line. Make sure that there are two forward slashes (//) at the begining of the line.

[ramppi]

Aditya,

I had both '/' but it was something to do with 'spaces'. I copy/pasted the code snippet from forum. And then it gave those string errors.
After tabulating it once more (taking off the 'white space' + adding it by tab)) line after line the string error moved line by line also (310,311..) ... and corrected.
(problem was only in the first snippet). Funny, cause when looking, you can't see any difference. But so it went.

Thank You for Your time Aditya

Matti

[wprowe]

In this block of code:

Code Select Expand

    if (isset($iptc) && is_array($iptc)) {
        if (isset($iptc['Title'])) $info[$lang_picinfo['iptcTitle']] = trim($iptc['Title']);
        if (isset($iptc['Copyright'])) $info[$lang_picinfo['iptcCopyright']] = trim($iptc['Copyright']);
        if (isset($iptc['Keywords'])) $info[$lang_picinfo['iptcKeywords']] = trim(implode(" ",$iptc['Keywords']));
        if (isset($iptc['Category'])) $info[$lang_picinfo['iptcCategory']] = trim($iptc['Category']);
        if (isset($iptc['SubCategories'])) $info[$lang_picinfo['iptcSubCategories']] = trim(implode(" ",$iptc['SubCategories']));
    }

Find the lines that reference "implode", change the "isset" to "!empty" at the beginning to fix the error you are seeing. I did that for mine and it resolved that error message.

[Absoblogginlutely]

Is there an announcement mailing list that is available so that I can be warned that there are problems like this rather than finding it out because I happened to see a post on another website? Either an email list or a rss feed would be great. The Rss feed on sourceforge for announcements doesn't mention this security hole.

[Nibbler]

You can subscribe to the announcments thread if you go to here and then click 'notify'.

[stilgar]

Hi ! You should probably mention the version change to 1.3.4 in the Changelog. Would have saved me 20 min diffing  1.3.3 and 1.3.4... 
 
edit:
that sounded a bit harsh maybe. i realize you have better things to do than work on 1.3 . Thanks for the great work and all the info on the forum!

[Rickshaw Driver]

In this block of code:

Code Select Expand

    if (isset($iptc) && is_array($iptc)) {
        if (isset($iptc['Title'])) $info[$lang_picinfo['iptcTitle']] = trim($iptc['Title']);
        if (isset($iptc['Copyright'])) $info[$lang_picinfo['iptcCopyright']] = trim($iptc['Copyright']);
        if (isset($iptc['Keywords'])) $info[$lang_picinfo['iptcKeywords']] = trim(implode(" ",$iptc['Keywords']));
        if (isset($iptc['Category'])) $info[$lang_picinfo['iptcCategory']] = trim($iptc['Category']);
        if (isset($iptc['SubCategories'])) $info[$lang_picinfo['iptcSubCategories']] = trim(implode(" ",$iptc['SubCategories']));
    }

Find the lines that reference "implode", change the "isset" to "!empty" at the beginning to fix the error you are seeing. I did that for mine and it resolved that error message.

Thanks, this fix worked.  Can someone from the dev team confirm that this fix is safe to use?  I am not a programmer and don't know what this actually does to the code.  Thank you.

[DJMaze]

To fix the issues with arrays use

Code Select Expand


if (isset($iptc) && is_array($iptc)) {
if (isset($iptc['Title'])) $info[IPTCTITLE] = strip_tags(trim($iptc['Title'],"\x0..\x1f"));
if (isset($iptc['Copyright'])) $info[IPTCCOPYRIGHT] = strip_tags(trim($iptc['Copyright'],"\x0..\x1f"));
if (!empty($iptc['Keywords'])) $info[IPTCKEYWORDS] = strip_tags(trim(implode(' ',$iptc['Keywords']),"\x0..\x1f"));
if (isset($iptc['Category'])) $info[IPTCCATEGORY] = strip_tags(trim($iptc['Category'],"\x0..\x1f"));
if (!empty($iptc['SubCategories'])) $info[IPTCSUBCATEGORIES] = strip_tags(trim(implode(' ',$iptc['SubCategories']),"\x0..\x1f"));
}

This way you don't run the 'one level' foreach() on the array

[judyksp]

I have version 1.3.3 from Fantastico.  Fantastico was provided by my webhost (Voda Host).   I upgraded coppermine using the txt file you provided and renamed it.

Since the upgrade I can no longer go into my website for coppermine.  It says MySQL too many connection error.  What is wrong?

Judy

[artistsinhawaii]

judy,

That error messae has nothing to do with Coppermine and everything to do with your server.  These are usually temporary problems that will go away, it's just the number of connections to your host/server's MySQL server is greater than the number allowed.  If it happens too frequently, ask your hosting service about it.

Dennis

[eskan]

i huv a problm, i just upgrade the cpg but the vulnerability still working, or maybe is another.. dont know, well u can see the web http://www.canalgogo.com/ and the XSS http://www.canalgogo.com/displayimage.php?album=5%20&pos=3%22%3Eblablabla%3C/h1%3E

i have really update? or its another bug?
Thx for answering

[Aditya Mooley]

Yes at first glance, language selector has a potential for XSS atleast in 1.3.x version of CPG.
The problem seems to have been solved in 1.4.x

Immediate recommendation is, do not use language selectors.
We will investigate furthur and post the fix if necessory.

[kkerr]

Hello, I upgraded my original CPG 1.33 to the CPG 1.34 version available "with the fix" written into it. performed the update.php etc

Initially 
Warning: implode(): Bad arguments. in /var/www/cpg133/displayimage.php on line 334
Warning: implode(): Bad arguments. in /var/www/cpg133/displayimage.php on line 336

So I then renamed and replaced the displayimage.php in hopes it would help,this changed the error to:

Warning: implode(): Bad arguments. in /var/www/cpg133/displayimage.php on line 338

Thus, your suggestions are welcome.

[kkerr]

If it helps, here is the current related code I am using:


if (isset($iptc) && is_array($iptc)) {
        //Sanitize the data - to fix the XSS vulnerability - Aditya
        foreach ($iptc as $key=>$data) {
          $iptc[$key] = htmlentities(strip_tags(trim($data,"\x7f..\xff\x0..\x1f")),ENT_QUOTES); //sanitize data against sql/html injection; trim any nongraphical non-ASCII character:
        }
        if (isset($iptc['Title'])) $info[$lang_picinfo['iptcTitle']] = trim($iptc['Title']);
        if (isset($iptc['Copyright'])) $info[$lang_picinfo['iptcCopyright']] = trim($iptc['Copyright']);
        if (!empty($iptc['Keywords'])) $info[$lang_picinfo['iptcKeywords']] = trim(implode(" ",$iptc['Keywords']));
        if (isset($iptc['Category'])) $info[$lang_picinfo['iptcCategory']] = trim($iptc['Category']);
        if (!empty($iptc['SubCategories'])) $info[$lang_picinfo['iptcSubCategories']] = trim(implode(" ",$iptc['SubCategories']));
    }