V230808 error using s2if shortcode

#1

2023/08/08 14:16:13 [error] 31142#31142: *8781 FastCGI sent in stderr: “PHP message: PHP Fatal error: s2If, unsafe conditional function [ get_user_option(s2member_auto_eot_time) ] in /var/www/openmtbmap.org/htdocs/wp-content/plugins/s2member/src/includes/classes/sc-if-conds-in.inc.php on line 194” while reading response header from upstream, client: 2001:4dd1:6992:0:f9ad:b826:475b:46d1, server: openmtbmap.org, request: “GET /de/support/members/ HTTP/2.0”, upstream: “fastcgi://unix:/run/php/php8.2-fpm.sock:”, host: “openmtbmap.org”

or
2023/08/08 12:53:05 [error] 31141#31141: *3114 FastCGI sent in stderr: “PHP message: PHP Fatal error: s2If, unsafe conditional function [ get_user_option(s2member_auto_eot_time) ] in /var/www/openmtbmap.org/htdocs/wp-content/plugins/s2member/src/includes/classes/sc-if-conds-in.inc.php on line 194” while reading response header from upstream, client: 114.119.152.119, server: openmtbmap.org, request: “GET /it/support/members/?_s2member_vars=file…level…1…file…odbl/nonunicode/velovietnam_gmapsupp.7z…Lz9zMm1lbWJlcl9maWxlX2Rvd25sb2FkPW9kYmwvbm9udW5pY29kZS92ZWxvdmlldG5hbV9nbWFwc3VwcC43eg%3D%3D&_s2member_sig=1642824505-ff1eeadc1f13eebac93ac7f0b8f29930 HTTP/1.1”, upstream: “fastcgi://unix:/run/php/php8.2-fpm.sock:”, host: “openmtbmap.org”, referrer: “https://openmtbmap.org/en/support/members/?_s2member_vars=file..level..1..file..odbl/nonunicode/velovietnam_gmapsupp.7z..Lz9zMm1lbWJlcl9maWxlX2Rvd25sb2FkPW9kYmwvbm9udW5pY29kZS92ZWxvdmlldG5hbV9nbWFwc3VwcC43eg%3D%3D&_s2member_sig=1642824505-ff1eeadc1f13eebac93ac7f0b8f29930”

1 Like
#2

The following change is the problem:

If I change line 151 back or to:
if(!is_multisite() || !c_ws_plugin__s2member_utils_conds::is_multisite_farm() || is_main_site() && in_array(strtolower($conditional), $blog_farm_safe))

site with [s2If get_user_option(s2member_auto_eot_time)]
will not crash.

Or go back to the old line is also fine.

If you don’t use that shortcode the new version is safe - however it will crash all sites that use certain s2if shortcodes.

2 Likes
#3

Actually it seems to happen on some other s2if funtions as well. That line of code clearly has some problems.

1 Like
#4

Additionally - this functionality and all the code that belongs to it really should be removed from s2member. Such functions are not acceptable in wordpress anymore. Whoever needs that should use must use plugins instead or use a shortcode snippet plugin where you write the php code somewhere in the plugin, which creates a shortcode for you that you can insert to have the php code executed. Plain PHP code in pages is a huge security risk and not allowed in wordpress anymore (as well as ezphp and similar plugins of course not acceptable anymore) (from s2member --> general restrictions)

Arbitrary PHP Code via [s2If php=""]

By default, the [s2If] Shortcode is limited to a specific set of Conditional Tags provided by WordPress and the s2Member plugin; e.g., [s2If current_user_can(access_s2member_level1)] ; as one quick example. Arbitrary PHP code is not allowed with this syntax. However, a second syntax variation exists; where it is possible to use arbitrary PHP code (only if enabled below). The second syntax variation uses one php Shortcode Attribute to run a single conditional check; e.g., [s2If php="is_user_logged_in() && current_user_can('access_s2member_ccap_music')"] . For developers, this has some obvious advantages. The code inside the php attribute is evaluated at runtime, so it’s possible to accomplish more when necessary. Of course, you could also use a plugin like ezPHP to accomplish the same thing (if you prefer).

1 Like
#5

Maybe have a setting to choose if you want safe or unsafe?

#6

Nope, actually plugins providing direct php insertion aren’t allowed on wordpress.org anymore.

#7

Oh, that’s what you mean with not acceptable. Not acceptable is different that non allowed.

I am not a native English speaker, though. :grimacing:

#8

Likely that code of S2member is too convoluted so it didn’t get discovered yet. Or maybe it doesn’t even work anymore.

I’m not fully sure it’s not allowed anymore, but all plugins that offered it as main purpose got taken down 4-5 years ago already.

1 Like
#9

Weird. I think I have one that I use to add filters (is that how it’s called?) to s2Member amongst other things.

#10

I had to fix that line, it was a security issue. You got the error because you were using a function that wasn’t whitelisted for the s2If. That was the problem, that it allowed those to be used when it shouldn’t.

$blog_farm_safe = apply_filters('ws_plugin__s2member_sc_if_conditionals_blog_farm_safe',
	array('is_user_logged_in', 'is_user_not_logged_in',
		'user_is', 'user_is_not', 'user_can', 'user_cannot',
		'current_user_is', 'current_user_is_not', 'current_user_can', 'current_user_cannot',
		'is_admin', 'is_blog_admin', 'is_user_admin', 'is_network_admin',
		'is_404', 'is_home', 'is_front_page', 'is_singular', 'is_single', 'is_page',
		'is_page_template', 'is_attachment', 'is_feed', 'is_archive', 'is_search',
		'is_category', 'is_tax', 'is_tag', 'has_tag', 'is_author', 'is_date',
		'is_day', 'is_month', 'is_time', 'is_year', 'is_sticky', 'is_paged',
		'is_preview', 'is_comments_popup', 'in_the_loop', 'comments_open',
		'pings_open', 'has_excerpt', 'has_post_thumbnail'), get_defined_vars());

get_user_option is not in that list. Of course it’s not wrong for you as the admin to use it, but the way it was it allowed others to do it if they can use shortcodes (e.g. editors). That line was wrong and now it’s fixed.

The implementation needs to change, because you were using it in a way that shouldn’t have worked (not your fault, and not obvious)… But there’s a couple of ways to use s2If with custom functions:

The list of allowed functions can be extended via the ws_plugin__s2member_sc_if_conditionals_blog_farm_safe filter. https://s2member.com/kb-article/hacking-s2member-plugin-w-hooksfilters-for-wordpress/

It’s also possible to use custom PHP conditions in the php attribute added by the Pro add-on. WP Admin s2Member Pro > Restriction Options > Simple Shortcode Conditionals > Allow Arbitrary PHP

[s2If php="get_user_option('s2member_auto_eot_time')"]

I did notice that I have created a couple of other conditional functions for s2If in previous releases (i.e. current_user_days_to_eot_less_than and current_user_gateway_is, that are not in that list. I’m adding those now.

1 Like
#11

Awesome! I remember reading about the gateway (I use ccaps for it as it wasn’t available before) but I didn’t pay attention to the fact you added the counter for eot. That’s actually really useful to add a notice somewhere on my site.

1 Like
#12

Christ!
So we did an update to the site that will crash s2?

Didn’t we have enough issues with the PayPal things?

For those of us who stupidly updated our s2, will everything crash, OR is there a list of short codes that will crash things if we use them now
(i.e., what short codes NOT to use if we don’t want s2 member to crash

  • and please God don’t tell me any of this is in the PayPal and pro-forms section?)

Sid

( Or is the issue fixed? Having trouble following this thread :slightly_smiling_face:)

1 Like
#13

If you use a shortcode that is not whitelisted it will crash the site with the shortcode.not the full website.

#14

It has nothing to do with the pro-forms or PayPal lol

If the s2If shortcode is used with a function that is not allowed, it now gives an error, only on the page where it was used, not the whole site.

That was the default behavior Jason gave it, it just wasn’t being triggered, and so other functions could be used, potentially making some PHP execution available to anyone that could use a shortcode (e.g. editors). It was a vulnerability that had to be closed.

But since it was allowing other functions, some found it useful with other conditionals, and now those are triggering the error. Not many have gone outside the basic conditionals, Felix and some others are more advanced users.

It’s still possible to use those conditionals, they just have to use the php attribute in s2If, or whitelist the additional functions.

I’m really sorry it inconvenienced some people, but I had to fix it.

I think that Jason’s intention behind the error, was to make it obvious in some way to the administrator (e.g. from the error log) that someone had tried using it that way.

I will look at changing the behavior, though, maybe instead of an error, just not parse it. If I do that, then it won’t leave a record, but maybe that’s alright, since it already wasn’t all these past years since the shortcode was created.

:slight_smile:

1 Like
#15

Well it’s not really advanced to show the EOT on user profile page, but only if user has EOT, or checkout options only to members that have an eot but hide them for those with active subscription that would be overwritten.
It’s actually basic things, but those are not easy to do with S2member. If you offer both subscription and fixed time accounts it’s actually a must to do. But yeah to avoid such conditions other membership plugins only offer subscription or lifetime membership. No fixed time…

Oh yeah and most people don’t show warning in error log but only error level… It should pop a warning in wordpress dashboard.

1 Like
#16

I had this code: [s2If cmbEsStripe() …

It calls to this function:

function cmbEsStripe() {
return eval(‘return (’.trim(do_shortcode(’’[s2Get constant=“S2MEMBER_CURRENT_USER_SUBSCR_GATEWAY” /]’ == “stripe”’)).’);’);
}

And I get this error:

Fatal error: s2If, unsafe conditional function [ cmbEsStripe() ] in /home/mmmkofti/coachingrealista.com/wp-content/plugins/s2member/src/includes/classes/sc-if-conds-in.inc.php on line 194

I don’t know what to do with this:

The list of allowed functions can be extended via the ws_plugin__s2member_sc_if_conditionals_blog_farm_safe filter. https://s2member.com/kb-article/hacking-s2member-plugin-w-hooksfilters-for-wordpress/

Allow Arbitrary PHP was already set to yes

I’d thank so much your help to know which of the fixes should I apply?

Thanks

#17

Thanks for the notice and the workaround. I have just updated 5 minutes ago and my site crashed :frowning:

#18

To use arbitrary PHP in s2If, you’d use the php attribute, e.g.:

[s2If php="cmbEsStripe()"]

:slight_smile:

#19

What is the file that stores the $blog_farm_safe array?
I tried github to find it via search but no success…

#20

Same file: s2member/src/includes/classes/sc-if-conds-in.inc.php

:slight_smile:

1 Like