tep_output_string vs tep_output_string_protected

[jonesevan007]

Background: When auth.net declines a credit card the visitor is taken back to the checkout_payment page with an error in the URL that is then parsed out and displayed. I wanted to add a helpful link to the error message so the visitor could learn more information about their error.

 

In order do use html (like <a href="">) I needed to downgrade the cleansizng function that displayed the error string from output_string_protected to just output_string.

 

Can you forsee any security consequences of making that one change to allow double quotes?

 

Thanks!

- Evan

									  <!-- Error Handling -->
									<?php
									  if (isset($HTTP_GET_VARS['payment_error']) && is_object(${$HTTP_GET_VARS['payment_error']}) && ($error = ${$HTTP_GET_VARS['payment_error']}->get_error())) {
									?>
								  <tr>
										<td>
											<table border="0" style="margin: 0px auto 0px auto;" width="100%" cellspacing="1" cellpadding="2" class="infoBoxNotice">
												<tr class="infoBoxNoticeContents">
													<!--<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><?php //echo tep_output_string_protected($error['title']); ?></td>-->
													<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><?php echo tep_output_string($error['title']); ?></td>
												</tr>
												<tr class="infoBoxNoticeContents">
													<!--<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><?php //echo tep_output_string_protected($error['error']); ?></td>-->
													<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><?php echo tep_output_string($error['error']); ?></td>
												</tr>
											</table>
										</td>
									</tr>
									<?php } ?>

[♥ecartz]

Can you forsee any security consequences of making that one change to allow double quotes?

Yes. The recommended way to do this would be to pass an extra parameter in the URL or to parse the string when it reaches you and then add the link. Passing an arbitrary link in the URL is itself a security risk, as it allows someone to link to your site, force a login, and then have the user click back to their site with validation from your site. Not to mention the possibility of any number of basic cracks to get the session ID via cross site scripting.

[jonesevan007]

<!--quoteo(post=1443667:date=Sep 30 2009, 11:36 PM:name=jonesevan007)--><div class='quotetop'>QUOTE (jonesevan007 @ Sep 30 2009, 11:36 PM) <a href="index.php?act=findpost&pid=1443667"><{POST_SNAPBACK}></a></div><div class='quotemain'><!--quotec-->Can you forsee any security consequences of making that one change to allow double quotes?<!--QuoteEnd--></div><!--QuoteEEnd-->Yes. The recommended way to do this would be to pass an extra parameter in the URL or to parse the string when it reaches you and then add the link. Passing an arbitrary link in the URL is itself a security risk, as it allows someone to link to your site, force a login, and then have the user click back to their site with validation from your site. Not to mention the possibility of any number of basic cracks to get the session ID via cross site scripting.

 

Thanks for the response. That's what I was thinking too. Any idea how to make an IF this error message THEN display this link for more information?

 

I'm liking the new theme of these forums :P

[♥FWR Media]

If the error string is entirely trusted ( you absolutely know the source (payment processor ) and therefore trust the incoming string ) then the output with limited protection is fine.

 

However the emphasis is on trusted have you REALLY done enough checks to ensure that the incoming is from the correct source and trustworthy?

 

And even then you should be running specific checks to check validity .. e.g. ok so it is an expected <a href, is the domain of the url within the <a href as expected?

[jonesevan007]

If the error string is entirely trusted ( you absolutely know the source (payment processor ) and therefore trust the incoming string ) then the output with limited protection is fine.

 

However the emphasis is on trusted have you REALLY done enough checks to ensure that the incoming is from the correct source and trustworthy?

 

And even then you should be running specific checks to check validity .. e.g. ok so it is an expected <a href, is the domain of the url within the <a href as expected?

 

The error message is drawn from the authorize.net payment contribution language file so I have complete control of what it outputs and the link URL. Path: includes/languages/english/modules/payment/authorizenet_aim.php

define('MODULE_PAYMENT_AUTHORIZENET_AIM_TEXT_ERROR', '<a href="#">Click to learn more</a>'.'There was a slight problem processing your Credit Card. ');

 

A simple solution that I'm thinking of is that I could put a generic "learn more" link for every payment error like:

<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><a href="www.google.com">sample link</a><?php echo tep_output_string_protected($error['title']); ?></td>

[♥FWR Media]

The error message is drawn from the authorize.net payment contribution language file so I have complete control of what it outputs and the link URL. Path: includes/languages/english/modules/payment/authorizenet_aim.php

define('MODULE_PAYMENT_AUTHORIZENET_AIM_TEXT_ERROR', '<a href="#">Click to learn more</a>'.'There was a slight problem processing your Credit Card. ');

 

A simple solution that I'm thinking of is that I could put a generic "learn more" link for every payment error like:

<td colspan="5" class="main" style="border: 0px solid black; font-weight: none;"><a href="www.google.com">sample link</a><?php echo tep_output_string_protected($error['title']); ?></td>

 

if the output is coming from a controlled file then you shouldn't be worrying .. yes we still do tep_output_string_protected which means we are using htmlspecialchars to protect the user but that is fine.

[jonesevan007]

if the output is coming from a controlled file then you shouldn't be worrying .. yes we still do tep_output_string_protected which means we are using htmlspecialchars to protect the user but that is fine.

 

Sweetness. Thanks FWR.