Daniel Iser
Forum Replies Created
-
@nigelgutzmann – Ok so I did some more digging & looked at a few older reports in our email & facebook that weren’t quite the same but showed strangeness.
Found that we had a fallback in case pumselect2 wasn’t found for some reason. It appears that in some edge cases (3+ copies of Select2 loaded) the timing of script loads could have somehow triggered it to fallback even if ours was loaded.
In the case I was able to test (Popup Maker + WP Fusion + ACF Pro fields on popup) I did notice some CSS changes where the fields were being set up using the wrong CSS classes (missing pum prefix).
I couldn’t work out how that was happening as the on change functionality was using our version of pumselect2, but the css classes appeneded to elements clearly came from the others.
I have removed that fallback as it shouldn’t ever be necessary and would indicate our code wasn’t loading correctly anyways.
That MIGHT resolve your issue, though I’m not really certain as it was hard enough to narrow down as it was. The only thing it changed for my tests was that the dropdowns appeared farther down the page than they should. They were still there but not positioned correctly due to class mismatches.
Hope that is helpful in the end result. If not would need more access to debug directly on your end.
Following up as this needs to be merged & released.
@dexzhou – Should be simple enough to convert the saved UTC time to local time when we render there, made a note just now for future update.
Appreciate the report.
Just to be sure, tested on clean install just now, couple screenshots. No issues editing popups, and you can see clearly that their copy of Select2 is loaded.
https://share.wppopupmaker.com/YEuDNxoN
https://share.wppopupmaker.com/z8ulpzZ8@nigelgutzmann – I think you’ve proven our case in your opening message, these conflicts are not coming from our code directly.
Why does Popup Maker not supply its own version of select2, which will definitely work, rather than relying on other plugins to supply select2, which could be any version (and thus potentially incompatible with Popup Maker)?
You linked to “related issues”, and the one from 2016 goes over the issue, and the very first response covers that we already rewrote it exactly as your quote above suggests, 7 years ago..
I wrote it this way for exactly the reason your saying is now our fault, to avoid any possible conflicts. The only way our pumselect2 interferes with ACF Pro is if they also rewrote select2 and happened to use the exact same name pumselect2. Highly unlikely.
And 700k installs later, we don’t see mass reports of issues, which we would as a decent portion of our user base also uses ACF
Also nearly all of our sites use ACF Pro & many of our extensions without issues in Popup interfaces or ACF interfaces.
Some follow ups though to try and narrow things down a bit further.
1. Do you have ACF custom fields for popups?
2. Have you considered a 3rd plugin that only does something when ACF is active, thus disabling ACF does make it go away leading to the assumption ACF or PM is the cause.- This reply was modified 3 years, 7 months ago by Daniel Iser.
@azadjohar – Any particular reason, there is nothing about them that could be exploited. They store
1as a value, can’t be exploited ever really as we check that they exists and are set to1, otherwise popup opens.I think you may have used a scanner or tool and its giving you false alerts or misleading info.
First setting
httponlywould literally break all intended usage as that makes it impossible for JavaScript to read them, and since that is the only way we check them to disable your popups properly its required not to be set.Lastly
secureis only used for sending data to the server, which again we never do.These cookies are set & read in the clients own browser only, never read on the server. and in no way security concerns.
Hope that clarifies, if you do try to do it, just know it will break things.
Take care.
@pipoulito – Are you trying to make all new popups use that trigger, or all popups in general, new and existing, use that trigger?
IE filter defaults, or filter popup settings passed to the JavaScript?
@scottalanmyers – Your rules must match that endpoint url somehow. For example maybe you chose “Is A Page” and that is actually a page pretending to be an endpoint. Not sure how its set up exactly, but if it was actually using the normal API endpoint mechanisms built into WP, our actions/filters would not even run to check permissions.
So they must be using a page or similar to represent that url, thus resulting in weirdness if you don’t account for it in your targeting rules.
It might be a question that requires asking them how theirs works in the context of you need to target rules to not match it.
Hope that helps.
@tlsoup – We have looked to offer that a few times before, but the complexities and hackish nature of the solutions required are not ideal.
Properly protecting the actual media files has 2 ways of being accomplished.
1. At the server level, modifying your servers configs in a way to block access directly to the files. This has the limitation of not being able to validate against a users profile/user-role. This method is very performance based, but also hard to allow flexibility.2. Via PHP, replace all usage of image1.jpg with hhkalw4h5laeufojdsklah.jpg, then have a DB table that maps that hash image to a real one. Then PHP can be used to validate if the current user has permission to view that image and return a fake image if not.
This is probably what you hoped for, BUT this makes a huge impact on your servers performance. Every protected image requires an entirely separate PHP process. This means every image will load the entirety of WordPress and all plugins, make multiple queries to the database and finally return the image.
If you had a page with 50 images, you would be making 51 separate page load requests to your server every time that page loads. If 10 people try to load it at once, now your talking 500 requests. This very quickly overwhelms most low cost servers making it not really viable.
Though we can implement this, we would have to recommend against most of our users enabling it without having their hosting upgraded to handle the extra loads that causes.
We are always re-evaluating, so it could get there one day, we definitely understand the desire for such a feature.
@wilcosky – Hmm that is strange, I’ve seen duplicate fields before with UM, but I don’t know that its caused by Content Control.
https://share.wppopupmaker.com/2Nu74Jvn
https://share.wppopupmaker.com/yAuQOXkmThat said the only way it could be rendered twice is if the action that we hook into to render them is called twice.
That would mean another plugin/theme is also likely adding custom fields there and doing it in a way that appends a second call to that action.
Maybe try troubleshooting mode?
@noelawill – I mean if your comfortable hacking your own solution in PHP I might be able to point you to some of our functions that could be used to manually check a specific restriction set as a condition to another filter. I haven’t confirmed it would be doable, but worth a shot.
That is you could theoretically filter the forms rendering methods (not familiar with those, you’d have to consult their docs), and not render it if that restriction set were to fail.
@ndukesx – Not a problem. Very much appreciate the report & confirmation. If you tell me what scanner you are using we might be able to add some comments that prevent it from throwing a notice.
@ndukesx – Appreciate the report. First in he future we have a dedicated form for these to make sure they get handled and fixed without alerting hackers to their presence here: https://github.com/PopupMaker/Popup-Maker/security/policy
Quick note, the data in question is
sanitizedwhen its saved. However I’m assuming you meantescapingdata which.html()does not do (stripping outscripttags)That said can you provide any attack details on how this could be exploited, or was this the result of an automated scanner notice?
I ask because I have gone over the ways that code gets called, and they all come from either our own hard coded messages, or from options only a user with admin privilege’s (
manage_options) already could edit. If a user can modify those messages, they have far more control than they could gain from trying to exploit this.The function with that code is only called during usage of our own Subscription form shortcode during submission to render error notices & success messages. It is also one of the ways admins output script tags for google analytics tracking on successful submission, so stripping out script tags isn’t exactly desirable behavior assuming they never took advantage of our extensions.
I’m happy to investigate as needed, especially if you , but potentially breaking X number of sites ability to track their submissions with no real security threat would not be the best path forward.
If you do have an exploit, and details on duplicating it, I implore you to use the link I posted above: https://github.com/PopupMaker/Popup-Maker/security/policy
@noelawill – I’m assuming your making restrictions in the settings page, and there are rules for forms.
First, those are not rules we are familiar with, so either another plugin has written custom code to add those, or my more realistic guess is that WP Forms is using post types to store their forms, and for whatever reason have not marked them private. Our condition system automatically generates rules for every non-private post type.
But those rules are applied during rendering of the page on the front end via normal the_content and template_redirect filters/actions.
I’m assuming they woulnd’t use those to render forms.
Our other plugin Popup Maker is similar though marked private, we use post types, but not the normal rendering stuff of WP.
Likely those targeting options shouldn’t be there.
Forum: Plugins
In reply to: [Performance Lab] PHP Error Cannot use object of type stdClass as array@jamesosborne – I tried to duplicate it just now, only non passed health check is inactive plugins, no warnings.
That said I logged it this weekend in Fatal Error Notify during some testing, not exactly sure how.
If nothing else maybe throw a sanity check that whatever that line is, is in fact an array. Will keep an eye on it.