Ticket #470 (closed defect: fixed)

Opened 2 years ago

Last modified 4 months ago

GetEnv is not a good idea (take 2)

Reported by: DarTar Owned by: JavaWoman
Priority: high Milestone: 1.3
Component: core Version: 1.1.6.2
Severity: major Keywords: security configuration
Cc:

Description (last modified by JavaWoman) (diff)

Follows from #98

Temporary fix applied to the 1.1.6.3 branch in [434].

1.1.7 requires a more structural approach.

Change History

Changed 2 years ago by JavaWoman

  • status changed from new to assigned
  • description modified (diff)

Changed 2 years ago by JavaWoman

(In [624]) Security (inheriting,if not exactly migrating, from 1.1.6.3) is the main focus here:

  • a little more cleanup (whitespace etc.)
  • first take on a more a structural implementation of the 1.1.6.3 solution for #98 (insecure GetEnv), addresses #470 (but needs more work: the intention is to provide an admin with an easy mechanism to:
    • point to a configuration file outside the web root (security); and
    • override some paths in general so some files can be shared between different Wikka installations on the same server
  • implementation of a new little function to encapsulate different methods of object instantiation in PHP4 (explicit pass by reference needed) and PHP5 (automatic pass by reference); this should prevent any grumbles from PHP5 that '=&' is deprecated. Used within this file, yet to be implemented elsewhere
  • introduced some new constants - more needed probably
  • a little reorganization

Changed 2 years ago by JavaWoman

(In [662]) "Relocatable Wikka" - part 1

This is the core of the functionality to make Wikka's code and configuration relocatable. See  http://wikkawiki.org/RelocatingWikka for background and detailed description. WARNING: trunk will be highly unstable until all parts of "Relocatable Wikka" have been committed! refs #470, #545

  • wikka.php:
    • simple mechanism with 'key files' to enable working in 'development mode' (NOTICES shown) and/or 'debug mode' (debug messages shown) without needing to edit the source
    • major restructuring - each section clearly demarcated with comments, and dependencies indicated; do NOT change this order unless you have an intimate understanding of what's happening here, and why!
    • two little function moved from Compatibility.lib.php to here, to enable evaluating some path overrides before including some libraries, including Compatibility.lib.php (!)
    • all paths to relocatable (groups of) components are now derived as CONSTANTS; consequently in the rest of Wikka (including the installer) either these constants (or the configuration values overriding the defaults they set) must be used instead of using string constants!
  • Compatibility.lib.php:
    • two little functions moved to wikka.php
    • other minor updates necessary for the relatabale functionality
  • Config.class.php:
    • new variables added
    • some new docblocks added where they were still missing
    • to enable the relocation functionality, several variables now (must!) pick up their default value from constants derived in wikka.php
    • a few now-obsolete variables commented out
  • override.config.php (new file):
    • this file includes the (commented-out) constants that may define possible overrides
    • every constant carefully commented
    • file docblock explains purpose, usage and has some tips

Changed 2 years ago by JavaWoman

(In [663]) "Relocatable Wikka" - part 2

Some changes in the (en) language file

refs #470, #545

  • en.inc.php:
    • a few texts promoted to general (WIKKA_) constants
    • a few constants were moved to wikka.php
    • obsolete constants uncommented
    • several minor textual changes
    • comments, todo markers

Changed 2 years ago by JavaWoman

(In [664]) "Relocatable Wikka" - part 3

The Wikka core class Wikka.class.php

refs #470, #545

  • updated/added comments and @todo/@@@ markers
  • added constant WIKKA_URL_EXTENSION to use in building paths
  • added object variables for URL and URL components
  • Query(): extended error reporting with MySQL error number (easier to look up)
  • CheckMySQLVersion(): modified to make use of getMysqlVersion() (in Compatibility.lib.php) and version_compare(): interface changed: now only needs a single string representing the baseline version to compare against
  • IncludeBuffred(): removed @todo since the $path parameter now originates from a path component derived and validated in wikka.php
  • ReturnSafeHTML():
    • uses (possibly relocated!) path from configuration
    • uses instantiate() for PHP-version-dependent method to return an object reference
  • LoadWantedPages2(): fixed bug occurring when called with no $sort parameter specified
  • FullTextSearch(): makes use of new interface for CheckMySQLVersion() (including in commented-out section in case we ever want to re-instate that)
  • Redirect(): no longer uses config "base_url" for default (obsoleted by relocation functionality) but derives default URL from new object variable "wikka_path" (see above, and Run() below)
  • Href(): no longer uses config "base_url" and literal 'wikka.php?wakka=' to start building URL, but new object variable "wikka_path" instead (see above, and Run() below)
  • StaticHref(): rewritten to comply with relocatability:
    • parameter $filename renamed to $filepath to more clearly designate its function; it can now be:
      • a relative path
      • an absolute path (starting with a slash)
      • a fully-qualified URL (in which case the input is simply returned)
    • docblock updated
    • makes use of new object variables "base_domain_url" and "base_url" to build a URL (see above, and Run() below)
  • LogReferrer(): no longer uses config "base_url" but object variable "base_url" (see above, and Run() below) to build its regex to find external referrer
  • Action(): uses renamed config value "wikka_action_path" (which now may be a relocated path)
  • Handler(): uses renamed config value "wikka_handler_path" (which now may be a relocated path)
  • SaveACL(): slight change to how the "values list" for the query is built to make it easier to turn this into a function
  • CloneACLs(): slight change to how the "values list" for the query is built to make it easier to turn this into a function
  • Run() - several changes:
    • restructured, all major steps are now also commented with a short description
    • added a section to derive (once!) URL and URL component object variables so they can be used directly instead of being derived repeatedly all over the place; this also obsoletes the configuration value "base_url"
    • removed now-obsoleted redirects for images and stylesheets

Changed 2 years ago by JavaWoman

(In [665]) "Relocatable Wikka" - part 4

The installer.

Note: although it's apparent quite a bit of work still needs to be done on the new installer, i've done my best to ,limit my changes to only those needed for the new relocation functionality; I have added various todo markers though.

refs #470, #545

  • inc/functions.inc.php:
    • update_default_page():
      • interface: instead of building paths with string literals, it now gets passed two extra parameters with location of default pages for default language, and a fallback path for the system default language
      • changed 'txt_filename' to 'txt_filepath' to avoid confusion with an actual file name
    • Language_selectbox():
      • uses constant WIKKA_LANG_PATH (which may contain a related path) instead of literal 'lang'
  • index.php:
    • use constant WIKKA_SETUP_PATH instead of string literal
  • default.php:
    • use constant CONFIG_DEFAULT_LANGUAGE for the system default language instead of string literal
  • check.php:
    • use already-defined constants MINIMUM_PHP_VERSION and MINIMUM_MYSQL_VERSION instead of PHP_REQ and MYSQL_REQ
    • make sure result of @touch() is evaluated and any error reported
  • setup.php:
    • directly use constant SITE_CONFIGFILE (which contains the -possibly relocated - path for the site configuration file) instead of a variable
    • make sure result of @touch() is evaluated and any error reported
    • report intended path for site config file before testing, so user can always check the intended path is being used (not only on error)
  • install.php:
    • derive variables for location of default pages for default language, and a fallback path for the system default language
    • pass these as extra parameters to the function update_default_page() (see above)
    • added pointer to future handling of page-specific ACLs for default pages will be needed for split up of UserSettings (refs #79)
    • directly use constant SITE_CONFIGFILE (which contains the -possibly relocated - path for the site configuration file) instead of a variable
  • links.php:
    • removed error_reporting(E_ALL): replaced by usage of 'errors' key file to trigger development mode - see wikka.php commit notes for [662]!
  • writeconfig.php
    • no longer writes $configbase_url? = $url (OBSOLETE now: Run() handles this dynamically)
    • no longer use $configbase_url? but internal variable $url instead (which holds the correct value)
    • minor correction to regex for deriving $rewrite_base
    • directly use constant SITE_CONFIGFILE (which contains the -possibly relocated - path for the site configuration file) instead of a variable

Changed 2 years ago by JavaWoman

(In [666]) "Relocatable Wikka" - part 5

Actions and not-really action.

refs #470, #545

Actions:

  • colour.php:
    • uses new name 'wikka_action_path' instead of 'action_path' for configuration value to build include path
  • emailpassword.php:
    • uses new object variable "wikka_url" instead of OBSOLETE configuration value "base_url" to build path to reference in email
  • mindmap.php:
    • use new config variable "freemind_uripath" instead of string literal to build URI path to the Freemind applet
    • removed needless span on error message
  • rss.php:
    • use new config variable "onyx_path" instead of string literal to build URI path to the Onyx-RSS class
    • removed needless span on error message
  • textsearch.php:
    • use new interface for CheckMySQLVersion(), passing one version string instead of three values

Not-really action:

  • header.php:
    • <base> element no longer needed (with correct usage of StaticHref())
    • consequently we no longer need the config variable "base_url" (let alone its replacement)

Changed 2 years ago by JavaWoman

(In [667]) "Relocatable Wikka" - part 6 (last of this set)

Handlers.

refs #470, #545

  • comments.xml.php:
    • use new config variable "feedcreator_path" (which may contain a relocated path) instead of string literal to build URI path to the FeedCreator class
    • uses instantiate() for PHP-version-dependent method to return an object reference
    • uses new object variable "base_url" instead of OBSOLETE configuration value "base_url" to use as source location
  • delete.php:
    • reformatted queries (slightly - needs more work)
    • uses new object variable "base_url" instead of OBSOLETE configuration value "base_url"
  • edit.php:
    • commented out dummy generation of header and footer since links aren't tracked there any more anyway
    • uses new config variable "wikiedit_uripath" (which may contain a relocated path) instead of string literal to build URI paths to the WikiEdit scripts
    • uses new config variable "wikiedit_uripath" (which may contain a relocated path) to build the variable $wikiedit_imgpath to pass as fourth parameter to the init() function of the WikiEdit object, so it can find its own images even if relocated. Note: WikiEdit already had this fourth parameter; it was just unused till now!
  • mindmap_fullscreen.php:
    • use constants WIKKA_LANG_PATH and CONFIG_DEFAULT_LANGUAGE to build paths to language file for selected default language and fallback language file (using system default language)
    • use constant ERROR_LANGUAGE_FILE_MISSING (in wikka.php) to generate error message if no language file can be found
    • use new config variable "freemind_uripath" instead of string literal to build URI path to the Freemind applet
    • a little cleanup (whitespace, naming)
  • recentchanges.xml.php:
    • use new config variable "feedcreator_path" (which may contain a relocated path) instead of string literal to build URI path to the FeedCreator class
    • uses instantiate() for PHP-version-dependent method to return an object reference
    • uses new object variable "base_url" instead of OBSOLETE configuration value "base_url" to use as source location
  • recentchanges.xml.php (note that even with the listed i18n changes i18n is not yet complete for this handler):
    • use FIRST_NODE_LABEL instead of string literal (i18n - refs #340)
    • use new object variable "wikka_url" instead of (OBSOLETE) config variable "base path" and constant WIKKA_URL_EXTENSION to build link to the current page
    • use WIKKA_ERROR_CAPTION and WIKKA_ERROR_ACL_READ_INFO instead of string literals to build error message (i18n - refs #340)
    • considerable cleanup (whitespace, coding guidelines)

Changed 2 years ago by JavaWoman

  • status changed from assigned to closed
  • resolution set to fixed
  • description modified (diff)

With changesets [662], [663], [664], [665], [666] and [667], work on this ticket (which targets relocation of the site configuration file only) is complete. Closing this ticket now. (Tested with a config file shared between two Wikka instances, and with a config file outside the webroot.)

With this set of changesets complete, trunk should be (reasonbly ;)) stable again.

Note that for its parent ticket #545 (which targets relocation of many more Wikka components using the same mechanism) more work needs to be done, so that will remain open for now.

Changed 4 months ago by DarTar

  • milestone changed from 1.2 to 1.3

Retargeting to 1.3, this ticket has already been closed in trunk, from which 1.3 will be branched. Consider backporting urgent issues to 1.2.X

Note: See TracTickets for help on using tickets.