 |
|
Item14629: Unauthorized user can replace UserRegistration page without approval.
Priority: Security
Current State: Closed
Released In: 2.1.6
Target Release: patch
Current State: Closed
Released In: 2.1.6
Target Release: patch
Restrict edit of certain sensitive topic names - Foswiki 2.1.6
Add a configuration option that lists sensitive topic names that should never be created or edited by a non-admin user in any web. Initial list includes UserRegistration, GroupViewTemplate, DefaultWebStatistics. These are all conditionally INCLUDED from Main web. In addition, we should protect ChangePassword, ResetPassword and ChangeEmailAddress, as they could be used in attacks as well.Move customization back to INCLUDE from the System web. - Foswiki 2.2.0
Change the places where an INCLUDE is made from Main, to only look in the System web. This may require sites to move/rename topics upon upgrade to Foswiki 2.2Alternative 2.2.0 solution
- Create a new web, Systemconfig or some name TBD
- Config setting $Foswiki::cfg{LocalConfigWeb} mapped to %LOCALCONFIGWEB%
- Any conditional INCLUDEs would point to %LOCALCONFIGWEB%
- Upgrading sites that are happy with current config can just set it to Main
- Future changes, we could investigate installing non-default Extensions into LocalConfigWeb. This would make distro packaging, and container type deployments easier.
IRC Discussion
(08:59:28 AM) mode (+o gac410) by ChanServ
(09:16:31 AM) gac410: Hi MichaelDaum, uebera||, CDot thinks the registration issue I discovered is worthy of a CVE. Do you all concur?
(09:18:49 AM) gac410: I can probably get a release out very quickly, but the big question, is there anywhere else where we automatically substitute a Main.blah in place of a System.blah topic merely based on its existence
(09:37:12 AM) uebera||: I'm just browsing through the description. I guess a code review would be in order as well, but for consistency, I'd expect that other forms/topics like ChangeEmailAddress, ChangePassword, ResetPassword can also be overridden.
(09:37:34 AM) gac410: No, So far as I can see it's only UserRegistration.
(09:38:28 AM) gac410: I'm creating a new task specific for this issue. Item14629
(09:40:49 AM) ***MichaelDaum grepping for %INCLUDE.*USERSWEB
(09:41:26 AM) MichaelDaum: WikiGroups may be an issue as well
(09:42:13 AM) MichaelDaum: imagine you'd register a user called "Group View Template"
(09:42:32 AM) MichaelDaum: next is WebStatistics using a similar %INCLUDE pattern
(09:43:30 AM) gac410: Damn.
(09:45:31 AM) gac410: https://foswiki.org/Tasks/Item14629
(09:45:55 AM) MichaelDaum: how would you proceed once you owned Main.(UserRegistration|GroupViewTemplate|DefaultWebStatistics) ?
(09:46:37 AM) gac410: I suppose you could insert javascript to capture registered IDs/Passwords?
(09:46:58 AM) gac410: At best it's a DOS attack, in that you can shut down registration.
(09:47:03 AM) uebera||: Can't we simply add a check so that nobody can register a "name" which is equivalent to an existing page in Main, System?
(09:47:19 AM) gac410: We already do that.
(09:47:35 AM) gac410: The issue is if the site did not customize the topic, so Main.UserRegistration does not exist
(09:48:13 AM) MichaelDaum: DefaultWebStatistics might be even worse
(09:48:43 AM) MichaelDaum: as it is used by a cronjob and thus run with admin rights
(09:49:21 AM) gac410: Damn, even on Foswiki.org, our Main.UserRegistration is not protected. Any registered user can edit it.
(09:50:27 AM) gac410: Okay that's fixed
(09:53:44 AM) uebera||: We don't/did not use "Set ALLOWWEBCHANGE" to restrict access to Main to the admin group by default? (I'd expect that the registration process has admin rights, so the individual user topic should not be affected)
(09:54:35 AM) gac410: No, Main is always editable by registered users. eg, so they can create their <user>LeftBar topic
(09:55:28 AM) MichaelDaum: the registration process is executed as RegistrationAgent, not AdminUser
(09:56:21 AM) gac410: Right. And it *is* possible to restrict Main from general edit, but that does break the custom left Bar concept.
(09:59:04 AM) uebera||: Workaround would be to always generate a private <user>LeftBar topic which can be edited like the "main" <user> topic.
(09:59:06 AM) gac410: TBH we should split Usersweb into 3. Users (for registration only) Home (for the landing page) and LocalSystem for the configuration of *anything* Ideally it would be nice if LocalSystem was an automatic override for System anthing. But that is WAAAY too big for even 2.2
(09:59:12 AM) MichaelDaum: So what is the simples way to fix the security flaw, ignoring for a moment all alternatives that would break people's installs, or be more radical as a change.
(09:59:19 AM) uebera||: But that does not help with existing installations in case the Main web is used by everyone.
(09:59:58 AM) gac410: Okay. Simple fix for Registration is to change the INCLUDE for Main.UserRegistration to be System.SiteUserRegistration?
(10:00:27 AM) MichaelDaum: wouldnt that break any customizations already out there with a Main.UserRegistration in place?
(10:00:44 AM) gac410: Yes. Users would have to move/rename their Main.UserRegistration.
(10:01:37 AM) MichaelDaum: from what I see, any "easy" fix will be ugly.
(10:01:54 AM) MichaelDaum: like a hack to block creating these evil topics
(10:02:31 AM) uebera||: We could extend the "Set DENYTOPICCHANGE" concept so that a default value can be specified from System/Web preferences. But since the syntax changes, we'd need something like ALLOW/DENYWEBTOPICSCHANGE?
(10:03:09 AM) gac410: Ugh, any "quick" hack that touches ACLs is IMO really dangerous. They are complex enough as it is.
(10:03:34 AM) uebera||: Agreed. Also, it's said that plugins could "ignore" ACLs.
(10:04:25 AM) gac410: bin/configure "AdminOnlyTopics" = (List of wildcard topics that can never be edited by anthing other than Admin regardless of the web)
(10:04:38 AM) MichaelDaum: good idea
(10:04:51 AM) uebera||: Yes!
(10:05:28 AM) MichaelDaum: let's do that
(10:05:56 AM) gac410: Okay. I can work on it. It probably goes into the same place we but the "*" wildcard for ACls.
(10:06:22 AM) uebera||: Is it a lot of work to include the webs here? *.xyz, SpecialWeb.zzz? (feature creep, I know...)
(10:07:18 AM) gac410: IF topicname matches (...), and not user isAdmin, deny change.
(10:08:15 AM) uebera||: "change" includes creation, right?
(10:08:20 AM) MichaelDaum: y
(10:09:05 AM) gac410: yes but ... need to make sure it actually works that way. Usually create permition checks for web change permission iirc
(10:09:22 AM) gac410: permission
(10:09:43 AM) MichaelDaum: IF ("web.topic" matches adminOnlyTopicsRegex and not isAdmin) then deny
(10:10:28 AM) gac410: I was going to even only protecting on topic name regardless of web.
(10:10:38 AM) MichaelDaum: this actually is a prerequisite before entering the check of the change acl
(10:10:45 AM) gac410: yes
(10:12:02 AM) gac410: So for 1.1.x users, do we build a 1.1.11, or just tell them to make sure that Main.UserRegistration exists, and is protected.
(10:13:31 AM) gac410: Now as far as web statistics, and running as root. I'm not sure how any topic content could break out and do system damage.
(10:14:15 AM) gac410: any more than any topic content could do system damage, as the web server cgi scripts run with auth as well.
(10:16:45 AM) MichaelDaum: I checked the UI::Statistics: it does _not_ call expandCommonMacros or the like. it calls the plugin handlers on save though ...
(10:17:05 AM) gac410: So one other thing. This does also protect sensitive topics from attack. Like creating Sandbox.ChangePassword and sending someone an email with that topic link
(10:17:57 AM) gac410: So you can't fish by putting a well known name into a different web. But there is the possibility of a punycode attack, spelling ChangePassword with a different character set.
(10:18:17 AM) MichaelDaum: What would be the default setting of AdminOnlyTopics then?
(10:18:28 AM) gac410: So maybe certain critical functions should *only* function if the request is from the well known topic.
(10:19:14 AM) gac410: UserRegistration ChangePassword ResetPassword ChangeEmail WebStatistics
(10:19:29 AM) gac410: GroupViewTemplate
(10:20:24 AM) gac410: I'm not too worried about plugin handlers and statistics or anything else. If you can somehow install plugins into a site, then all bets are off.
(10:20:26 AM) uebera||: "well known topic" needs to include %REGPARTS%, though.
(10:21:19 AM) uebera||: And %NEWUSERTEMPLATE% (from https://foswiki.org/System/UserAuthentication#Custom_registration_page)
(10:23:33 AM) gac410: We should change the instructions to go back to only considering System for Foswiki 2.2 A minor update will be more disruptive. So users may have to edit some topics.
(10:24:17 AM) gac410: So 2.1.x, we block sensitive topic names, and in 2.2, we go back to using Systemweb for customization where practical.
(10:24:30 AM) gac410: (plus continue to block sensitive names)
(10:28:59 AM) uebera||: Sorry, but isn't that a minor update in any case?
(10:30:32 AM) gac410: Another idea. Since we allow customized registration topics in 2.x, how about a list of AuthorizedRegistrationTopics. A list of Web.Topic names that the register script will accept any input from. Explicit. System.UserRegistration, System.ChangePassword System.ResetPassword, System.ChangeEmail.
(10:30:46 AM) gac410: If you have customized UserRegistration, then add the custom names to that list.
(10:31:00 AM) gac410: That also prevents punycode replacements.
(10:31:17 AM) uebera||: And it allows for complete localisation including topic names.
(10:33:29 AM) gac410: That does not buy much though. If I create a punycode ChangePassword, which posts to a 3rd party site, nothing is protected.
(10:37:19 AM) uebera||: This should/can only be addressed by pointing users to client side protection mechanisms like https://chrome.google.com/webstore/detail/http-request-blocker/eckpjmeijpoipmldfbckahppeonkoeko?hl=en
(10:38:07 AM) uebera||: Else, we'd need a whitelist of sites and parse every page before delivery.
(10:38:32 AM) gac410: y, IMO not worth it.
index 9e7cc8a..a7f38c1 100644
--- a/core/lib/Foswiki.spec
+++ b/core/lib/Foswiki.spec
@@ -560,6 +560,11 @@ $Foswiki::cfg{TopicUserMapping}{ForceManageEmails} = $FALSE;
# for access permission, then it will not get blocked by these controls.
$Foswiki::cfg{AccessControl} = 'Foswiki::Access::TopicACLAccess';
+# **STRING LABEL="Sensitive Topic Names" **
+# A list of topic names that should never be created or edited by a non-admin
+# user. These topics will always be denied.
+$Foswiki::cfg{AccessControlACL}{RestrictedEdit} = 'UserRegistration,ChangePassword,ResetPassword,ChangeEmailAddress,GroupViewTemplate,Default$Foswiki::cfg{Stats}{TopicName}';
+
# **BOOLEAN LABEL="Enable Deprecated Empty Deny" EXPERT **
# Optionally restore the deprecated empty =DENY= ACL behavior.
# If this setting is enabled, the "Empty" =DENY= ACL is interpreted as
diff --git a/core/lib/Foswiki/Access/TopicACLAccess.pm b/core/lib/Foswiki/Access/TopicACLAccess.pm
index 8a599f9..f1f306f 100644
--- a/core/lib/Foswiki/Access/TopicACLAccess.pm
+++ b/core/lib/Foswiki/Access/TopicACLAccess.pm
@@ -98,6 +98,14 @@ sub haveAccess {
my ( $allow, $deny );
if ( $meta->{_topic} ) {
+ if ( $Foswiki::cfg{AccessControlACL}{RestrictedEdit} =~
+ m/\b\Q$meta->{_topic}\E\b/ && $mode ne 'VIEW' )
+ {
+ $this->{failure} = $session->i18n->maketext('access denied on topic');
+ print STDERR 'a ' . $this->{failure}, "\n" if MONITOR;
+ return 0;
+ }
+
$allow = $this->_getACL( $meta, 'ALLOWTOPIC' . $mode );
$deny = $this->_getACL( $meta, 'DENYTOPIC' . $mode );
-- GeorgeClark - 16 Feb 2018
Patch for Foswiki 1.1.10 (Should fit older 1.1, though line displacements are different.) Note that I've also revised the 2.x patch
diff --git a/lib/Foswiki.spec b/lib/Foswiki.spec
index 58b44e8..39822a5 100644
--- a/lib/Foswiki.spec
+++ b/lib/Foswiki.spec
@@ -723,6 +723,11 @@ $Foswiki::cfg{INCLUDE}{AllowURLs} = $FALSE;
# DEPRECATED</font> - use <a href="http://foswiki.org/Extensions/SafeWikiPlugin">SafeWikiPlugin</a> instead.
$Foswiki::cfg{AllowInlineScript} = $TRUE;
+# **STRING LABEL="Sensitive Topic Names" **
+# A list of topic names that should never be created or edited by a non-admin
+# user. These topics will always be denied.
+$Foswiki::cfg{AccessControlACL}{RestrictedEdit} = 'UserRegistration,ChangePassword,ResetPassword,ChangeEmailAddress,GroupViewTemplate,Default$Foswiki::cfg{Stats}{TopicName}';
+
# **BOOLEAN EXPERT**
# If a login name (or an internal user id) cannot be mapped to a wikiname,
# then the user is unknown. By default the user will be displayed using
diff --git a/lib/Foswiki/Meta.pm b/lib/Foswiki/Meta.pm
index 11a01d3..529cc8e 100644
--- a/lib/Foswiki/Meta.pm
+++ b/lib/Foswiki/Meta.pm
@@ -1754,6 +1754,13 @@ sub haveAccess {
my ( $allow, $deny );
if ( $this->{_topic} ) {
+ if ( $Foswiki::cfg{AccessControlACL}{RestrictedEdit} =~ m/\b\Q$this->{_topic}\E\b/ && $mode ne 'VIEW') {
+ $this->{failure} =
+ $session->i18n->maketext('access denied on topic');
+ print STDERR 'a ' . $this->{failure}, "\n" if MONITOR_ACLS;
+ return 0;
+ }
+
my $allow = $this->_getACL( 'ALLOWTOPIC' . $mode );
my $deny = $this->_getACL( 'DENYTOPIC' . $mode );
diff --git a/lib/Foswiki/UI/Register.pm b/lib/Foswiki/UI/Register.pm
index 4ca59b6..2a12c09 100644
--- a/lib/Foswiki/UI/Register.pm
+++ b/lib/Foswiki/UI/Register.pm
@@ -1431,6 +1431,19 @@ sub _validateRegistration {
);
}
+ if ( $Foswiki::cfg{AccessControlACL}{RestrictedEdit} =~ m/\b\Q$data->{WikiName}\E\b/) {
+ $session->logger->log( 'warning',
+"Registration rejected: Topic $Foswiki::cfg{UsersWebName}.$data->{WikiName} restricted name."
+ );
+ throw Foswiki::OopsException(
+ 'attention',
+ web => $data->{webName},
+ topic => $session->{topicName},
+ def => 'bad_wikiname',
+ params => [ $data->{WikiName} ]
+ );
+ }
+
#new user's topic already exists
-- GeorgeClark - 16 Feb 2018
Foswiki 1.1.x has an additional issue. It doesn't catch the error caused when Registration attempts to save the restricted topic. The user ends up in .htpasswd and can log in, but does not have a user topic, or an entry in WikiUsers. Essentially a phantom user.
A hardcoded reject of certain names is probably the easiest solution. ... See above
-- GeorgeClark - 16 Feb 2018
ItemTemplate edit
| Summary | Unauthorized user can replace UserRegistration page without approval. |
| ReportedBy | GeorgeClark |
| Codebase | 2.1.5, 2.1.5 RC, 2.1.5 beta, 2.1.4, 2.1.4 RC1, 2.1.3, 2.1.3 beta2, 2.1.3 beta1, 2.1.2, 2.1.1, 2.1.0, 2.1.0 beta1, 2.0.3, 2.0.2, 2.0.1, 2.0.0, 2.0.0 RC, 1.2.0 beta2, 1.2.0 beta1, 1.1.10, 1.1.9, 1.1.9 RC, 1.1.8, 1.1.7, 1.1.6, 1.1.6 dev, 1.1.5, 1.1.5 RC2, 1.1.5 RC1, 1.1.4, 1.1.4 RC2, 1.1.4 RC1, 1.1.4 beta2, 1.1.4 beta1, 1.1.3, 1.1.3 RC1, 1.1.3 beta1, 1.1.2, 1.1.1, 1.1.0, 1.1.0 beta1 |
| SVN Range | |
| AppliesTo | Extension |
| Component | TopicUserMappingContrib |
| Priority | Security |
| CurrentState | Closed |
| WaitingFor | |
| Checkins | distro:38a74eb43e0f distro:da712a6c844e distro:d88c95d38488 distro:83c28bd8cf10 |
| TargetRelease | patch |
| ReleasedIn | 2.1.6 |
| CheckinsOnBranches | Release02x01 master Item14288 |
| trunkCheckins | |
| masterCheckins | distro:38a74eb43e0f distro:da712a6c844e distro:d88c95d38488 distro:83c28bd8cf10 |
| ItemBranchCheckins | distro:38a74eb43e0f distro:da712a6c844e distro:d88c95d38488 distro:83c28bd8cf10 |
| Release02x01Checkins | distro:38a74eb43e0f distro:da712a6c844e distro:d88c95d38488 |
| Release02x00Checkins | |
| Release01x01Checkins |
Edit | Attach | Print version | History: r8 < r7 < r6 < r5 | Backlinks | View wiki text | Edit wiki text | More topic actions
Topic revision: r8 - 20 Jan 2020, MichaelDaum
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. 
Legal Imprint Privacy Policy
Legal Imprint Privacy Policy