My plugin removed from WP.org extend directory

[Jeffro]

I've learned a lot from this forum topic.

This was the primary reason for reinstating the thread in hopes of helping other people who might stumble across it.

[Otto]

That seems slightly overkill, since the data would presumably be santised before entry. On flicking through the code originally I assumed it was running the sanitisation before input, but it seems I was wrong. I guess it's better to go over-kill on security than under-kill though. By double sanitising it I guess it will make it twice as hard to penetrate. Or is there perhaps some way to hack through it without the output sanitisation?

I prefer to do my validation of data on entry. I talk about this on my Settings API Tutorial, but basically I try not to save anything to the database that is untrusted.

I don't bother to sanitize the output. If it's in the database, then I consider it to be a trusted source. Because if the database is untrustworthy, you're already hacked.

[Ryan]

I don't bother to sanitize the output. If it's in the database, then I consider it to be a trusted source. Because if the database is untrustworthy, you're already hacked.

Do you (or anyone else here) know if my understanding that the DIFF posted above is just sanitising the output?


Your blog post seems to do pretty much exactly what I've done in my latest plugin, although I also white listed my options which I don't think you did. I haven't gone totally crazy about validating them either, which I might go do this morning just to be on the safe side. My current setup just validates them by checking a whole bunch of options with various possible values by checking there's no nasty HTML in there, whereas I should probably be checking each value individually for their exact specifications (some of them contain HTML, some contain only numbers, some contain only "on" or "off" etc. but at the moment I'm using the same script for each one to confirm that they're clean, which is probably not a good idea since I'm not confirming they're exactly what they should be.

I might also validate some of my outputs too. It may be overkill, but I'd rather cover my bases in case I somehow let something slip through the cracks.

[andreasnrb]

I've been taught to do output sanitation. "I" also do validation before submitting a form both client and server side and give feedback to the user. The framework I usually develop with does this for you based on some configs.

Really should update the plugin I have in the directory. It ain't exactly up to par.

[Otto]

Do you (or anyone else here) know if my understanding that the DIFF posted above is just sanitising the output?

Looks like it's doing a lot of sanitizing for field outputs on the main site. This would prevent possible XSS attacks if somebody got their mal-stuff into the back-end options. Escaping that stuff on save makes more sense to me though. Still, whatever works, can't hurt I suppose.

Careful about double escaping things though. Do it one place or the other, not both. Weird stuff happens when you do it more than once.

Your blog post seems to do pretty much exactly what I've done in my latest plugin, although I also white listed my options which I don't think you did.

The register_setting function causes the whitelisting stuff to happen automatically, in combination with the use of settings_fields.

If you use separate options instead of one array of options, then you have to register_setting each of them. The array of options approach generally makes more sense. It also makes for easier cleanup with an uninstall.php file (one delete_option and you're done).

I haven't gone totally crazy about validating them either, which I might go do this morning just to be on the safe side.

That's where I do tend to go nuts. For radios/checkboxes, I ensure that the values are in a given set of possibles using in_array. For text fields that don't require open-ended entry, I validate the hell out of them using regexp's. For more open-ended fields, I run them through html strips, esc_attrs, and so forth, depending on content. And so on. Every option must get validated on save.

[Ryan]

The register_setting function causes the whitelisting stuff to happen automatically, in combination with the use of settings_fields.

Ummm, well that's interesting ...

That would certainly explain why there is next to no documentation whatsoever on how to use white listing.

[Ryan]

Hmmm, I'm running to problems.

esc_attr() appears to strip out " characters and replace them with " which is no good as I'm trying to allow users to enter CSS in, which may use " characters.

I'm now just using stripcslashes(). Any ideas if there is a more suitable function to use in this situation?

[andreasnrb]

esc_attr is just for title, alt attributes etc. Can you use " in class names?

[Ryan]

esc_attr is just for title, alt attributes etc.

Ah, that makes sense based on it's name. The codex page just says what it strips out and it seems that the fixes for the LightSEO plugin uses it to filter out a textarea's content, which is what I'm using it for here, specifically for entering CSS code.

Can you use " in class names?

No, but you can in CSS which is what I'm trying to filter.


I think wp_filter_nohtml_kses() is what I need to use, so I'll plow ahead and use that unless anyone suggests otherwise.

[andreasnrb]

K I thought you were perhaps filtering the class="" attribute. Which made the comment about " odd =).

I think wp_filter_nohtml_kses() is what I need to use, so I'll plow ahead and use that unless anyone suggests otherwise.

Hmm can the users write > then?