Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-10428

optionget/optionset functions in session.php and acp_users.php incorrectly check whether $data is at its default value

    Details

      Description

      Both functions (either as method of class user of or as method of class acp_users) in question are defined this way (simplified):

      function optionget( ... $data = false)
      {
      	...
      	$var = ($data) ? $data : $this->data['user_options'];
      	...
      }

      The problem in here is that $data will also evaluate to FALSE if it was handed over nonetheless. Consider a case where I use the $data parameter by giving the value 0 (ordinal zero) or '' (empty string) - this value wouldn't be used, instead

      $this->data['user_options']

      will be used. Which is fatal.

      A correct approach would be to test without invoking typecasts:

      function optionget( ... $data = false)
      {
      	...
      	$var = ($data !== false) ? $data : $this->data['user_options'];
      	...
      }

      I submitted this as minor bug because right now no code in phpBB uses the $data parameter, so nothing is affected. Only exception: the STK uses it in /tools/admin/reparse_bbcode.php, but obviously nobody ever had problems with it.

        Activity

        Hide
        AmigoJack AmigoJack added a comment -

        I forgot: optionset() comes with the same issue. Again: no code in phpBB currently makes use of the $data parameter, even not the STK.

        Show
        AmigoJack AmigoJack added a comment - I forgot: optionset() comes with the same issue. Again: no code in phpBB currently makes use of the $data parameter, even not the STK.
        Hide
        bantu Andreas Fischer added a comment -

        Sounds like a valid issue to me. Include in 3.0.10 if patch is provided, otherwise 3.0.11.

        Show
        bantu Andreas Fischer added a comment - Sounds like a valid issue to me. Include in 3.0.10 if patch is provided, otherwise 3.0.11.
        Hide
        bantu Andreas Fischer added a comment -

        It also should not use the cache $this->keyvalues when $data is specified.

        Show
        bantu Andreas Fischer added a comment - It also should not use the cache $this->keyvalues when $data is specified.
        Hide
        Oleg Oleg [X] (Inactive) added a comment -

        Not tested at all but I made both suggested changes, I think.

        Show
        Oleg Oleg [X] (Inactive) added a comment - Not tested at all but I made both suggested changes, I think.

          People

          • Assignee:
            Oleg Oleg [X] (Inactive)
            Reporter:
            AmigoJack AmigoJack
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development