Is this a security breach ?

Carlo_13

Hi,

I am not 100% sure, but for forums using facebook/twitter/tagging plugin, i think anybody can change the setting just by adding to the url:

settings/tagging

or

settings/facebook

or

settings/twitter


Is this the same for you guy ?

ddumont

I am allowed to change the Facebook appid on this forum.

ddumont

Tagging gives me a permission problem. I didn't try twitter yet.

Carlo_13

Oh gosh,

I have just tried and you're right. And when you saved, is it effective ?

Is there a way to block the access to non admin users ?

ddumont

I don't want to change the config here. I didn't try to.

ddumont

I've submitted a patch in my pull request
https://github.com/vanillaforums/Garden/pull/897
@Todd @Tim @Lincoln @Mark

Carlo_13

I've submitted a patch in my pull request
https://github.com/vanillaforums/Garden/pull/897
@Todd @Tim @Lincoln @Mark

Tks

Linc

Thanks for the heads up, I'm turning on the bat signal.

crmarks

Should I manually apply @ddumont's patches to my 2.0.17.9 installation?

Linc

You can safely apply this change to fix the flaw: https://github.com/ddumont/Garden/commit/042ffc3bf4779febd55eb4e7c0dd76d02df9645e

Linc

I have applied @ddumont's patch to unstable branch.

crmarks

The correction will bring up a login page if someone tries to submit the form, but it will still display the data to someone who isn't logged in. Still perhaps a security issue?

ddumont

Maybe need another check in the view as well?

Linc

@crmarks I suspect the page is cached for you. Try a different browser or hard-refresh/clear your browser's cache.

crmarks

My bad, I placed the new line after, rather than before,

if ($Sender->Form->IsPostBack()) {

On my twitter plug, now it immediately goes to a login prompt. Problem solved!

Thanks Lincoln!

Todd

Thanks for the fixes guys. I uploaded a new version at http://vanillaforums.org/addon/vanilla-core. This will address the problem.

ddumont

@Todd it may be helpful for application developers if the permissions declared in the plugin info section were automatically enforced so that it wouldn't be up to the plugin developer to place this check in the plugin settings method.

Is it currently supposed to work like that?
What's the "SettingsPermission" => "Garden.Settings.Manage" option do if not this? Would this be a bug, or feature that got overlooked?