search

Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 800 forks source link

Sync: clarify use of base64 encoding #24842

Open jeherve opened 2 years ago

jeherve commented 2 years ago

It would be nice if we could expand our inline documentation in that file: https://github.com/Automattic/jetpack/blob/afce3f6e68346ec2de554c37b4fbf05dfdd5fd7f/projects/packages/sync/src/class-json-deflate-array-codec.php#L27-L45

The use of base64_encode and base64_decode is often flagged by security plugins (it's forbidden in WPCS for a reason), but we do not provide much explanation as to why we implemented things that way in the file. Site owners or plugin authors landing on that file would need more details to know why it is safe to keep using that plugin.

jeherve commented 2 years ago

cc @JulioPotier who brought this up to me just now.

anomiex commented 2 years ago

it's forbidden in WPCS for a reason

It's not actually forbidden in WPCS. The message WPCS produces is a warning,

%s() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.

The reason being that people do stupid things with it trying to obfuscate bad code, often enough for it to get flagged I guess. But it's perfectly valid for actually transporting data across a channel that might not be 8-bit clean.