Bugs in /admin/banner_manager.php and in functions_general.php ??

[angeloio]

Hi all,

I think there is a bug in admin/banner_manager.php in the
latest zen cart version:
Zen Cart 1.3.8a
Database Patch Level: 1.3.8
and I did some search in the forum and could not locate it somewhere else (I am not sure of that though...)...I apologize if it is reported earlier...

So the code

      case 'deleteconfirm':
        $banners_id = zen_db_prepare_input($_GET['bID']);

        if (isset($_POST['delete_image']) && ($_POST['delete_image'] == 'on')) {
          $banner = $db->Execute("select banners_image
                                 from " . TABLE_BANNERS . "
                                 where banners_id = '" . (int)$banners_id . "'");

          if (is_file(DIR_FS_CATALOG_IMAGES . $banner->fields['banners_image'])) {
           if (is_writeable(DIR_FS_CATALOG_IMAGES . $banner->fields['banners_image'])) {
              unlink(DIR_FS_CATALOG_IMAGES . $banner->fields['banners_image']);
            } else {
              $messageStack->add_session(ERROR_IMAGE_IS_NOT_WRITEABLE, 'error');
            }
          } else {
            $messageStack->add_session(ERROR_IMAGE_DOES_NOT_EXIST, 'error');
          }
        } 

If I am not mistaken to unlink something you must have write permission to the directory and NOT on the file itself. So we should check if the directory is writable. In the database the whole path is saved so for a solution I propose the following:

//if (is_writeable(DIR_FS_CATALOG_IMAGES . $banner->fields['banners_image'])) {
if (is_writeable(DIR_FS_CATALOG_IMAGES . substr($banner->fields['banners_image'], 0, strripos($banner->fields['banners_image'], '/')) )) { 

Please examine and confirm.
-----------------------------------------------
Also some "bugs" exist in function zen_trunc_string() in file functions_general.php where substr and strlen functions used are not utf-8 safe, so a utf-8 string is not cut correctly....

Proposed fixes:

  function zen_trunc_string($str = "", $len = 150, $more = 'true') {
    //--IA--
    mb_internal_encoding("UTF-8");
    if ($str == "") return $str;
    if (is_array($str)) return $str;
    $str = trim($str);
    // if it's les than the size given, then return it
    if (mb_strlen($str) <= $len) return $str;
    // else get that size of text
    //$str = substr($str, 0, $len);
    //--IA--
    $str = mb_substr($str, 0, $len);
    // backtrack to the end of a word
    if ($str != "") {
      // check to see if there are any spaces left
      if (!substr_count($str , " ")) {
        if ($more == 'true') $str .= "...";
        return $str;
      }
      // backtrack
      while(mb_strlen($str) && ($str[mb_strlen($str)-1] != " ")) {
        $str = mb_substr($str, 0, -1);
      }
      $str = mb_substr($str, 0, -1);
      if ($more == 'true') $str .= "...";
      if ($more != 'true' and $more != 'false') $str .= $more;
    }
    return $str;
  } 

Please also note that inside several modules (e.g. modules/featured_products) the product description is NOT returned using zen_trunc_string but using substr directly so another proposed fix is the following:

    //$products_desc = substr(strip_tags($featured_products->fields['products_description']), 0, 85) . '...';
        $products_desc = zen_trunc_string(zen_clean_html(stripslashes(zen_get_products_description($featured_products->fields['products_id'], $_SESSION['languages_id']))), PRODUCT_LIST_DESCRIPTION); 

The above should be applied in several modules.....please modify according to your preferences....

Please examine all the above and confirm.

I thank you so much for your patience and time

Best Regards to all the wonderful people here,

Ioannis Angelopoulos

[DrByte]

As to file vs folder permissions, while I agree that folder permissions typically indicate what may be done with the file, that is not an absolute certainty. You didn't say whether you're encountering specific problems with the issue and what those problems are, or whether you're just making a suggestion. If you feel it necessary to check the folder permissions, I would suggest *also* checking the file anyway.

Regarding the use of mb_xxxxxx() functions, feel free to use them if your server supports them. The majority of english-based webservers don't have any mb_xxxxx support compiled into their PHP configurations, and thus any attempts to use mb_xxxx() functions will result in fatal errors, and thus not provide any benefit at all.