Potencial XSS vulnerability

[chluo]

I am using osCommerce2 and find one potential XSS vulnerability in its version 2.3.4.1:

osCommerce implements function tep_db_query() to execute SQL statement.  In case of MySQL error, the function tep_db_query() would call tep_db_error() to handle the mysql errors:

$result = mysqli_query($$link, $query) or tep_db_error($query, mysqli_errno($$link), mysqli_error($$link));

The tep_db_error() function basically calls die() function to display the error back to users: 

die('<font color="#000000"><strong>' . $errno . ' - ' . $error . '<br /><br />' . $query . ' ...);

The $query variable is sent by users and is well sanitized against SQL injection. However, it will also be used in the die() function (a sensitive XSS function like echo()) when Mysql returns errors. In multiple files (e.g.,  "/admin/modules.php") ,  the $query variable is not sanitized (against XSS) and can be exploited because of the die() function.

I suggest adding XSS sanitizers in the tep_db_error() function to avoid this kind of attack.

 

[ruden]

https://github.com/ruden/vanilla-oscommerce/commit/753fbbf5cafe47a1035188cfc36078f6767e6970

[Jack_mcs]

@chluoYou may want to install this addon to prevent errors from being displayed to the customers.

[chluo]

5 minutes ago, Jack_mcs said:

@chluoYou may want to install this addon to prevent errors from being displayed to the customers.

Yes, this also works. But wouldn't it be better to fix it in a newer version because many other users use this app not only me 😀.

[Jack_mcs]

If by "newer version" you mean the version of oscommerce, that might be the case with the new planned version. The current V2 version is no longer supported so such changes would not be made to it.

[bonbec]

Excuse me for asking the question, but if the person already have an access to the admin, what's the use of doing an XSS inclusion?

[chluo]

16 minutes ago, bonbec said:

Excuse me for asking the question, but if the person already have an access to the admin, what's the use of doing an XSS inclusion?

The attacker can be a common user, the two query functions I mentioned are in the include folder.

 

 

[Hotclutch]

2 hours ago, ruden said:

https://github.com/ruden/vanilla-oscommerce/commit/753fbbf5cafe47a1035188cfc36078f6767e6970

Does this fix the reported problem?

[chluo]

20 minutes ago, Jack_mcs said:

If by "newer version" you mean the version of oscommerce, that might be the case with the new planned version. The current V2 version is no longer supported so such changes would not be made to it.

OK. Please let me know if it has been fixed. Thanks

[chluo]

1 minute ago, Hotclutch said:

Does this fix the reported problem?

I would suggest adding XSS sanitizers in https://github.com/ruden/vanilla-oscommerce/blob/dev/catalog/includes/functions/database.php at line 42. That is: die('<font color="#000000"><strong>' . $errno . ' - ' . $error . '<br /><br />' . htmlspecialchars($query) ...).

[Hotclutch]

I see the commits are for Vanilla osCommerce. If someone has a solution for regular osCommerce and the CE versions (which probably are the same) that would be useful.

[Hotclutch]

die('<font color="#000000"><strong>' . $errno . ' - ' . $error . '<br /><br />' . htmlspecialchars($query) . '<br /><br /><small><font color="#ff0000">[TEP STOP]</font></small><br /><br /></strong></font>');

@chluo you mean like this?

[chluo]

Just now, Hotclutch said:

die('<font color="#000000"><strong>' . $errno . ' - ' . $error . '<br /><br />' . htmlspecialchars($query) . '<br /><br /><small><font color="#ff0000">[TEP STOP]</font></small><br /><br /></strong></font>');

@chluo you mean like this?

Yes.

[Jack_mcs]

1 hour ago, Hotclutch said:

I see the commits are for Vanilla osCommerce. If someone has a solution for regular osCommerce and the CE versions (which probably are the same) that would be useful.

The goal is, or should be, to hide the failure from the visitor. That's what my addon does. 

[Hotclutch]

31 minutes ago, Jack_mcs said:

The goal is, or should be, to hide the failure from the visitor. That's what my addon does. 

The instructions for your addon say to :

1) - Upload the included osc_error_handler_db_handler.php file to
the root of your shop and then execure it by using a url like:

https://YOUR DOMAIN/osc_error_handler_db_handler.php

but there is no such file included, only osc_error_handler.php in the functions folder. Is that right?

[Hotclutch]

Sorry, nevermind, I see it under the database folder.

[Hotclutch]

I guess it would be advisable to do both - fix the vulnerability and install the addon.

Would it still be possible for a hacker to exploit this vulnerability with the addon installed? i.e a hacker who knows osCommerce and who already knows the vulnerability exists.

[Jack_mcs]

The addon doesn't stop attacks. It just prevents the message from displaying on the site. For a while a few years ago there was a common attempt where the hacker would enter an invalid command just so they could see the command. Once they had the format, they would put in valid data and try again. The shops code should handle such attacks but the older shops don't. But even in Phoenix, at least earlier versions, still displayed the error so that should be stopped regardless.

[ruden]

10 hours ago, chluo said:

I suggest adding XSS sanitizers in the tep_db_error() function to avoid this kind of attack.

Requests are used preparation, in order to perform injection you need to have access to the code.

It does not affect anything.

Delete code die('<font color="#000000"><strong>' . $errno . ' - ' . $error . '<br /><br />' . $query . '<br /><br /><small><font color="#ff0000">[TEP STOP]</font></small><br /><br /></strong></font>');