Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix security hole in AliasHandler #23

Merged
merged 3 commits into from Feb 8, 2017

Conversation

Janfred
Copy link
Contributor

@Janfred Janfred commented Feb 3, 2017

Without this fix it is possible to delete a protected alias via editing
the request parameter of the alias to delete.

A simple example: If the alias abuse@example.com is defined as default alias and the alias other@example.com exists, you can just change the parameter in the delete url:

The delete url for other@example.com will probably look like this:
https://pfadmin.example.com/delete.php?table=alias&delete=other%40example.com&token=<CSRF-Token>

You can just change the request to
https://pfadmin.example.com/delete.php?table=alias&delete=abuse%40example.com&token=<CSRF-Token>
and Postfixadmin will delete the default alias.

The suggested fix just adds a condition to the delete function in AliasHandler which will fail if $this->can_delete is false.

Without this fix it is possible to delete a protected alias via editing
the request parameter of the alias to delete.
@@ -441,6 +441,11 @@ public function delete() {
return false;
}

if ($this->can_delete) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thanks for reporting it!

However I wonder if this condition is the wrong way round - I'd think we need
if ( ! $this->can_delete) {
(note the added '!')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooops...
I've forgotten the ! while transferring the patch from my production system to my local git copy...

@cboltz
Copy link
Member

cboltz commented Feb 8, 2017

This is CVE-2017-5930.

@cboltz cboltz merged commit bf9ec09 into postfixadmin:master Feb 8, 2017
svn2github pushed a commit to svn2github/PostfixAdmin that referenced this pull request Feb 8, 2017
Thanks to Janfred, postfixadmin/postfixadmin#23


git-svn-id: svn://svn.code.sf.net/p/postfixadmin/code/trunk@1889 a1433add-5e2c-0410-b055-b7f2511e0802
@cboltz
Copy link
Member

cboltz commented Feb 8, 2017

I released PostfixAdmin 3.0.2 which includes this fix and some non-security bugfixes.

Thanks again for reporting this and for submitting the fix.

@xpunkt
Copy link

xpunkt commented Feb 9, 2017

would be nice to see 3.0.2 github tagged :)

but thumps up that it is on github, i would try to make a gentoo ebuild to get cutting edge

@DavidGoodwin
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants