Security fix: Improved escaping for JSON attributes and structured data

[timbocode]

Backport of woocommerce/woocommerce@7def966.

See #214.

Files changed

includes/class-wc-structured-data.php
includes/wc-formatting-functions.php
includes/admin/class-wc-admin-pointers.php
includes/admin/class-wc-admin-setup-wizard.php
includes/admin/class-wc-admin-assets.php
includes/admin/helper/class-wc-helper.php
includes/admin/meta-boxes/views/html-product-data-variations.php
includes/admin/reports/class-wc-admin-report.php
includes/admin/reports/class-wc-report-coupon-usage.php
includes/admin/reports/class-wc-report-customers.php
includes/admin/reports/class-wc-report-sales-by-category.php
includes/admin/reports/class-wc-report-sales-by-product.php
includes/admin/reports/class-wc-report-sales-by-date.php
includes/libraries/action-scheduler/classes/ActionScheduler_wpPostStore.php
includes/widgets/class-wc-widget-layered-nav.php
includes/api/legacy/v1/class-wc-api-json-handler.php
includes/api/legacy/v2/class-wc-api-json-handler.php
includes/api/legacy/v3/class-wc-api-json-handler.php
templates/single-product/add-to-cart/variable.php
tests/unit-tests/util/class-wc-tests-core-functions.php

One additional file was excluded:

includes/admin/helper/class-wc-helper-updater.php (see #218 (review) below)

[codecov-io]

Codecov Report

Merging #218 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master     #218      +/-   ##
============================================
- Coverage     40.82%   40.82%   -0.01%     
  Complexity    13447    13447              
============================================
  Files           367      367              
  Lines         51277    51286       +9     
============================================
  Hits          20936    20936              
- Misses        30341    30350       +9

Impacted Files	Coverage Δ	Complexity Δ
...admin/reports/class-wc-report-sales-by-product.php	0% <ø> (ø)	36 <0> (ø)	⬇️
...es/admin/reports/class-wc-report-sales-by-date.php	32.65% <ø> (ø)	24 <0> (ø)	⬇️
includes/admin/class-wc-admin-setup-wizard.php	0% <ø> (ø)	138 <0> (ø)	⬇️
includes/admin/reports/class-wc-admin-report.php	35.17% <0%> (ø)	102 <0> (ø)	⬇️
includes/wc-formatting-functions.php	74.63% <0%> (-0.92%)	0 <0> (ø)
...des/admin/reports/class-wc-report-coupon-usage.php	0% <0%> (ø)	34 <0> (ø)	⬇️
...cludes/admin/reports/class-wc-report-customers.php	0% <0%> (ø)	13 <0> (ø)	⬇️
includes/admin/class-wc-admin-pointers.php	0% <0%> (ø)	8 <0> (ø)	⬇️
includes/admin/class-wc-admin-assets.php	0% <0%> (ø)	46 <0> (ø)	⬇️
...dmin/reports/class-wc-report-sales-by-category.php	0% <0%> (ø)	42 <0> (ø)	⬇️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 548ee48...386ad47. Read the comment docs.

[bahiirwa]

This file includes/admin/helper/class-wc-helper.php Has functions we cleaned out as we made cc leaner by removing the woo services subscriptions and their API checks since we have no way of knowing what’s going on the other end of the line. Those still need to be out.

[timbocode]

Testing

The changes in this PR are related to escaping JSON attributes and structured data and it's difficult to carry out any meaningful manual testing. Nevertheless, I've carried out some testing including:

• Created new product with variations, tested on frontend
• Verified structured data output using Google's Structured Data Testing Tool
• Checked Reporting functionality

In addition, all existing Travis tests have passed.

Do we need to create any further automated tests?

[nylen]

I compared this diff against woocommerce/woocommerce@7def966 and all looks good apart from the two minor comments I noted above.

Two files were modified in the upstream commit but not here - includes/admin/helper/class-wc-helper-updater.php because we removed this file, and includes/admin/helper/class-wc-helper.php because we removed all of the code that was updated in the upstream commit. So the way this was done is correct.

Do we need to create any further automated tests?

I think if we do the following then we will be about as ready as we can be:

• Compare the diffs against the corresponding WC commits
• Keep the current Travis build passing
• Do more extensive manual testing after all security fixes are merged

[nylen]

Poor @SInCE gets a lot of pings 😂

This looks good to me.

[bahiirwa]

Manual Testing done affected files

• on setup using the admin wizard
• Setup products that are variable
• Get reports on sales

Tests pass.