Jump to content

Archived

This topic is now archived and is closed to further replies.

dadada

"CSRF Token - Admin Side" forum support topic

Recommended Posts

May you like to explain why this is good, and for what reason shopowners should add it...

 

$form .= tep_draw_hidden_field('token', $token) . PHP_EOL;

 

 

 

.


This is a signature that appears on all my posts.  
IF YOU MAKE A POST REQUESTING HELP...please state the exact version
of osCommerce that you are using. THANKS

 
Get the latest Responsive osCommerce CE (community edition) here

Share this post


Link to post
Share on other sites

@@burt: As you know programmers can do same work with different codes ;)

 

thanks for the trick, I'll include that in next release.

Share this post


Link to post
Share on other sites

I mean maybe you should explain what the problem is that your code cures.

 

Forget that line of code I just added, I added that after, by the edit.

 

Most shopowners have no clue what your code does or what potential problem it does cure.

 

 

 

 

 

.


This is a signature that appears on all my posts.  
IF YOU MAKE A POST REQUESTING HELP...please state the exact version
of osCommerce that you are using. THANKS

 
Get the latest Responsive osCommerce CE (community edition) here

Share this post


Link to post
Share on other sites

Similar to what was done for the Catalog side in v2.3, forms will also be protected with a session token on the Administration Tool to protect store owners clicking on external malicious links that could perform administrative actions if they have an active Administration Tool session in their browser. Such malicious links would also bypass any kind of htaccess protection layer due to the active session in the browser (browser tab or window).

 

There have been no reports of this happening - it is just a security blanket that is being added to v2.3.3.

 

@, the solution you came up with is a quick solution that adds the token to all internal links and is not desirable. This is what was done for the Catalog side to protect forms individually:

 

https://github.com/osCommerce/oscommerce2/commit/95b005d1312b636695ee278471496372e8d56fd3

 

This is what should be done for the Administration Tool as well.


:heart:, osCommerce

Share this post


Link to post
Share on other sites

@@harald: Thanks for your comments :)

 

But, your codes will work only at POST form submits, and never gonna prevent GET CSRF attacks.

 

And keep in mind that there are hundreds of addons may use get parameters to change the values.

 

 

#UPDATE 2:

 

Actually, I don't see any problem to make "token= ... " in all internal links.

If you know bank sites, other ecommerce sites , phpMyadmin, and other populer CMSs are using tokens as the same way.

Share this post


Link to post
Share on other sites

@@burt: Actually, I didn't understand what you're talking about ! Pls, explain better.

 

 

#update:

 

Alright, the reason for update to fix get forms. I already added the description into update log.

 

This bug has reported before few hours.

 

"Hi,

I added your addon the probleme is when i do a search in my admon i go back to login looks like the token is not registered any idea???"

Share this post


Link to post
Share on other sites

@@harald:

 

The information that you point is TRUE and let check it out the texts.

 

"CSRF tokens in GET requests are potentially leaked at several locations: browser history, HTTP log files, network appliances that make a point to log the first line of an HTTP request, and Referrer headers if the protected site links to an external site."

 

-My solution uses session based tokens, so after the log-out nobody cannot use again (ex: server log files, browser history, etc.).

-" My solution uses oscommerce framework's functions and cannot referrer to any external link. "

You can check it out too.

-And the admin is already free to limit session.gc_maxlifetime using php.ini file in the admin folder for better practice.

 

If sensitive server-side actions are guaranteed to only ever respond to POST requests, then there is no need to include the token in GET requests.

 

-My solution will work well with 3d party add-ons here.

 

Well :) , Unfortunately I don't have a time to comment more detailed. Actually I spent last 8-9 months for web application security. I've developed the security extension for X shopping cart.

 

As I'm no more working with/on oscommerce I've just shared my work with the community.

 

As you pointed out:

Note: Contributions are used at own risk.

 

And I can talk much more about oscommerce security issues.

 

example : Imagine a bot , which:

 

-go to create_account.php page finds name="formid" and submit forms using curl.

So the result : just in minutes database can crush.

 

Good Improvements:

-Use random value with random sizes like name="ndiu93y49nf9438gf9gb34" (+regenerate on server side on every success or/and fail)

-Use captcha, which cannot read by bots like reCaptcha." (+regenerate on server side on every success or/and fail)

-Use account activations, and use cron job to remove all unactivated accounts periodically to prevent spamming.

-Use limit for reviews etc...

-Use Capthca at logins (to prevent brute force attack and help preventing to DDOS) + form submits (which affect db) etc...

 

-Also another security issue at image uploading in admin side. that doesn't check mime tyoe of uploading file.

finfo_file can be a solution there but that will need phpv5.3.

 

And so on...

Sorry, I really don't have a time to tell everything about security. So sorry, may I cannot answer in to the next post.

 

Thanks to community.

Share this post


Link to post
Share on other sites

+ :) action recorder at user logins based on user emails. ie: if (email_exists(email) && password !== true) $attempt++;

Share this post


Link to post
Share on other sites

This is a good quote from the OWASP article:

 

Ultimately, the acceptance of this risk as opposed to the cost of significant architecture design is up to the business.

 

Having the token automatically added to all internal links poses minimum security risk of exposing it in such a public manner. It is more work to add tokens manually - especially for Add-Ons - however this provides more control over the use of the token.

 

We may disagree on the actual implementation, but agree that any kind of implementation should be added. :thumbsup:


:heart:, osCommerce

Share this post


Link to post
Share on other sites

@@Harald Ponce de Leon

 

Interesting conversation this and it's great to see CSRF protection added to osCommerce.

 

Instead of lines like: -

 

if (isset($HTTP_POST_VARS['action']) && ($HTTP_POST_VARS['action'] == 'process') && isset($HTTP_POST_VARS['formid']) && ($HTTP_POST_VARS['formid'] == $sessiontoken))

 

Would it make sense to have a tep function that validates forms, CSRF and typecasts?

 

Example: -

 

 $sessiontoken = 'dw3f53523f523534';
 $_POST = array( 'action'  => 'process',
			  'form_id' => 'dw3f53523f523534',
			  'input'   => '1',
			  'hack'    => '<script>alert(\'Nasty!\');</script>',
			  'escape'  => '&testing tep_ &' );
 // tep_form_validate( array, [,$csrf_protection = true], [,$csrf_only_protection = false])
 $result = tep_form_validate( array( 'action' => 'process',
								  'input'  => 'int',
								  'hack'   => 'no_tags',
								  'escape' => 'tep_output_string_protected' ), $csrf_protection = true, $csrf_only_protection = false );
 if ( false === $result ) {
   echo 'Form validation failed';
 } else echo '<pre>' . var_dump( $result ) . '</pre>';

 

Results in: -

 

array(4) {

["action"]=>

string(7) "process"

["input"]=>

int(1)

["hack"]=>

string(16) "alert('Nasty!');"

["escape"]=>

string(23) "&testing tep_ &"

}

 

Reason I ask is I just wrote one, one minute I was thinking about it, the next ....

 

Simple implementations: -

 

tep_form_validate( array(), true, true );

 

Checks only the CSRF and returns bool true/false

 

tep_form_validate( array( 'action' => 'process' ), true );

 

Checks CSRF and that _POST[action] = process and returns typecast array( 'action' => (string)process )

 

tep_form_validate( array( 'action' => 'process', 'some_string_number' => 'int' ), true );

 

1) Checks CSRF

2) That _POST[action] = (string)process

3) Type casts 'some_string_number' to int

4) returns typecast array( 'action' => (string)process, 'some_string_number' => (int)123 )

 

Currently typecasts are int, numeric, float, string, no_tags, array and tep_ ( use a tep_function on the string )

 

I thought it might be useful to have a relatively short function that checks CSRF, validates the _POST against what is expected then returns an array of type cast results.

Share this post


Link to post
Share on other sites

@@Harald Ponce de Leon

 

Well .. to be fair .. it is conceptual so I wanted your opinion before taking the time to go through those processes.

Share this post


Link to post
Share on other sites

Robert, it looks interesting! That's why I requested to see a working example :)

 

If it's a lot of changes, it won't go in v2.3.3, but could in v2.4 where legacy code will be removed/replaced.


:heart:, osCommerce

Share this post


Link to post
Share on other sites

Robert, it looks interesting! That's why I requested to see a working example :)

 

Right oh!

 

I'll have a bash .. I've only ever used SVN before and no public online services so it should be hilarious.

 

If I destroy github I'll claim diminished responsibility .. a touch of insanity .. then point them in your general direction :D

 

 

If it's a lot of changes, it won't go in v2.3.3, but could in v2.4 where legacy code will be removed/replaced.

 

Well I'll write it with a load of functionality which, if you happen to like it, you can obviously strip out as you see fit.

 

Obviously there are a lot of forms in 2.3.2 so equally obviously it would mean a lot of code changes if used for every form capture.

Share this post


Link to post
Share on other sites

Want to work on it? :)

 

Ooops :) That was bad idea. Why?

Because the attacker will be able to block victim's login. The similar thing happened before years at HOTMAIL.

So, captcha still is the better solution.

Share this post


Link to post
Share on other sites

I'll have a bash .. I've only ever used SVN before and no public online services so it should be hilarious.

 

If I destroy github I'll claim diminished responsibility .. a touch of insanity .. then point them in your general direction :D

 

Be sure to check out this classic presentation to help you get started:

 

http://blip.tv/oscommerce/working-with-the-development-repository-on-github-1933713


:heart:, osCommerce

Share this post


Link to post
Share on other sites

×