Item13185: Crash in Meta::getPreference() with certain malformed URLs.

pencil
Priority: Urgent
Current State: Closed
Released In: 1.2.0
Target Release: minor
Applies To: Engine
Component: FoswikiMeta
Branches: master
Reported By: GeorgeClark
Waiting For:
Last Change By: GeorgeClark
Recreate on trunk with: http://trunk.foswiki.org/bin/view/blah/Sandbox
  • The web name needs to be lower case
  • The topic name needs to exist as a web.

The issue occurs during Foswiki.pm parameter sanitization.

Around line 2036, $web = Foswiki::Sandbox::untaint( $web, \&Foswiki::Sandbox::validateWebName ) || ''; results in a blank $web name AFTER the web name has been defaulted, so earlier comments assert that $web must be defined at this point becomes untrue.

The second problem happens in block of code at line 2050: With the web name empty, and the Topic name actually existing as a web, this code ends up assigning a $web name, in this example, of "/Sandbox" and a topic name of "WebHome"

    #if the parsed per spec web&topic don't exist, try to help the user
    if ($topicSpecified) {
        if (  !$this->topicExists( $web, $topic )
            && $this->webExists( $web . '/' . $topic ) )
        {

#requested view/Web/Sub - when there is no Web.Sub topic, but there is such a web
            $web   = $web . '/' . $topic;
            $topic = $Foswiki::cfg{HomeTopicName};
        }

Because the webname is prefixed with a /, it seems to pass the UI checks, but then kills Meta.

I think it's better to leave the lowercase web in place, and let Foswiki::UI throw an error. But the condition would still exist anytime a "web" name completely fails validation, is sanitized to empty string, and the Topic also happens to be a valid webname.

Not sure where to go with this one. We're trying to be too helpful I suspect.

-- GeorgeClark - 02 Jan 2015

Can't call method "get" on an undefined value at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 679.
 at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 679.
   Foswiki::Meta::getPreference(Foswiki::Meta=HASH(0x8048e05a0), "DENYWEBVIEW") called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Access/TopicACLAccess.pm line 208
   Foswiki::Access::TopicACLAccess::_getACL(Foswiki::Access::TopicACLAccess=HASH(0x8045a13a8), Foswiki::Meta=HASH(0x8048e05a0), "DENYWEBVIEW") called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Access/TopicACLAccess.pm line 137
   Foswiki::Access::TopicACLAccess::haveAccess(Foswiki::Access::TopicACLAccess=HASH(0x8045a13a8), "VIEW", "GeorgeClark", Foswiki::Meta=HASH(0x8048c7b88)) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Meta.pm line 1923
   Foswiki::Meta::haveAccess(Foswiki::Meta=HASH(0x8048c7b88), "VIEW") called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 600
   Foswiki::UI::checkAccess(Foswiki=HASH(0x802db8048), "VIEW", Foswiki::Meta=HASH(0x8048c7b88)) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI/View.pm line 165
   Foswiki::UI::View::view(Foswiki=HASH(0x802db8048)) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 372
   Foswiki::UI::__ANON__() called at /usr/local/lib/perl5/site_perl/5.16/Error.pm line 421
   eval {...} called at /usr/local/lib/perl5/site_perl/5.16/Error.pm line 413
   Error::subs::try(CODE(0x80107f510), HASH(0x802e2fb28)) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 498
   Foswiki::UI::_execute(Foswiki::Request=HASH(0x802db4798), CODE(0x802d7a0f0), "view", 1) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/UI.pm line 324
   Foswiki::UI::handleRequest(Foswiki::Request=HASH(0x802db4798)) called at /usr/home/trunk.foswiki.org/core/lib/Foswiki/Engine/CGI.pm line 98
   Foswiki::Engine::CGI::run(Foswiki::Engine::CGI=HASH(0x80213f858)) called at /home/trunk.foswiki.org/core/bin/view line 29.

-- GeorgeClark - 02 Jan 2015

The root cause of this seems to be Item9225: distro:b32d3a7947c9 We're trying to be too helpful.

-- GeorgeClark - 16 Mar 2015

Bumping this to urgent. Not sure if we should revert the feature or try to fix it.

Testing a bit more. It's no longer possible to create a topic with the same name as a subweb. Instead, it attempts to create the existing WebHome within the Subweb. If the topic exists, it's accessible, but it cannot be created.

  • Sandbox/
    • Subweb/
      • Blahtopic
      • Blahtopic/

The above structure can be created and accessed easily on 1.1.9. However on trunk, once the "Blahtopic" web exists, it's not possible to create the topic Blahtopic.

Definitely a blocker. Foswiki.pm attempts to guess / fill in defaults for the web and topic. Invalid components are replaced with defaults, and the UI code never gets any indication that the guesses were made. So messages about invalid web names never get triggered. I believe that the code in Foswiki.pm that parses the path into web and topic components needs to be rewritten. Way too much hacking / guessing going on.

-- GeorgeClark - 16 Mar 2015

The more I dig into this the more confused I'm getting. My impression is that the following should work:

http://trunk.foswiki.org/bin/viewauth?defaultweb=System;topic=WebPreferences;web=Sandbox

The parameters seem to be (often?) ignored. Printing $this->{webName} & $this->{topicName}, it gets modified after return from line 2110 in Foswiki.pm:

WEB  System TOPIC WebPreferences 

    # SMELL: what happens if we move this into the Foswiki::Users::new?
    $this->{user} = $this->{users}->initialiseUser( $this->{remoteUser} );

WEB2 Usersweb TOPIC WebHome 

This appears to be a completely separate issue from the path issues. And also note the query parameter web is ignored. It's not even referenced in that part of Foswiki.pm.

I also wrote a modified path parser, which appears to address the issues, and everything seems functional until a REST script, which seems to trigger a 500 with no errors reported anywhere.

-- GeorgeClark - 17 Mar 2015

New parser checked in, along with some revised unit tests and additional checks in in some of the UI scripts. This needs careful review.

There is one ugly hack, it sets {topicName} and {webName} a 2nd time to resolve an issue where user->initialize() overlays the values. This should probably be an ASSERT along with some debugging to resolve the issue.

-- GeorgeClark - 19 Mar 2015

Resolved the hack. HomePagePlugin assumed that if there was no path_info, then it could override the web/topic. Needed to bail if topic= or defaultweb= params are provided.

-- GeorgeClark - 19 Mar 2015

Not sure why this is still marked for my feedback; discussed it somewhat with George on IRC. Removing myself (and adding George as working on) so I can focus limited time on other tasks.

-- CrawfordCurrie - 19 Mar 2015

Just looking for review. Closing the task. It was a new bug in trunk, not in 1.1.9.

-- GeorgeClark - 19 Mar 2015
 

ItemTemplate edit

Summary Crash in Meta::getPreference() with certain malformed URLs.
ReportedBy GeorgeClark
Codebase trunk
SVN Range
AppliesTo Engine
Component FoswikiMeta
Priority Urgent
CurrentState Closed
WaitingFor
Checkins distro:9838f1124604 distro:28ebc0008dce
TargetRelease minor
ReleasedIn 1.2.0
CheckinsOnBranches master
trunkCheckins
masterCheckins distro:9838f1124604 distro:28ebc0008dce
ItemBranchCheckins
Release01x01Checkins
Topic revision: r8 - 19 Mar 2015, GeorgeClark
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy