Link doesn't open in new page after setting it in the Item Description

Discussion in 'General Support' started by david62311, May 23, 2022.

  1. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    Hello everybody! I missed you all. I have a found a new bug to report. We can get back to work on the Webid Script now.

    This bug is in the item description on the sell.php page when we go to sell an item. This bug should be easy to fix. When I post a link in the item description I set the target to open in a new page like this in the photo below but, when someone clicks on a link it just navigates away from the page.

    Link New Window for Webid.jpg

    I go to sell an item.
    When I am in the Item Description I click the chain icon for the link
    I select target as> New Window (_blank). This is supposed to make the link open a new page but, it makes it navigate away from the page.

    When I go back and edit it and highlight the link and click the chain icon again, it reads <not set>
     
  2. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    I am lost. I thought this was going to be an easy one. This chkeditor is the problem but, where is the code with the drop down list to select the target of the url link? This seems like an external issue outside of the Webid script. I am just guessing there. I searched the Github for the words "not set" "new window" and it's not coming up in the script and it's not in my EN Language file.

    The chkeditor doesn't show much in the includes.

    I will look at this for a little bit. If you all want a crack at it then go ahead please.
     
  3. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
  4. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    This is the code in the en.js link that I just shared in post #3:

    I cleaned it up so, it could be read easier.

    Code:
    "options":
    "Options",
    "target":"Target",
    "targetNew":"New Window (_blank)",
    "targetTop":"Topmost Window (_top)",
    "targetSelf":"Same Window (_self)",
    "targetParent":"Parent Window (_parent)",
     
  5. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    So, when I right click in the editor and I use inspect it it gives me this inner html code:

    HTML:
    <select id="cke_85_select" class="cke_dialog_ui_input_select" aria-labelledby="cke_86_label" style="width : 100%">
    <option value="notSet"> &lt;not set&gt;</option>
    <option value="frame"> &lt;frame&gt;</option>
    <option value="popup"> &lt;popup window&gt;</option>
    <option value="_blank"> New Window (_blank)</option>
    <option value="_top"> Topmost Window (_top)</option>
    <option value="_self"> Same Window (_self)</option>
    <option value="_parent"> Parent Window (_parent)</option></select>
    When I search github, I don't see anything related to this id: cke_85_select
     
  6. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    I just manually updated the description in the database to make the link open in a new windows. So, we or I will have to trace down the code just before it inserts into the 'description' of the webid_auctions in the database. At that point we should be able to figure it out.
     
  7. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    I believe it's a security filters the cleanvars causing it to not allow it the extra link code to be selected...which is good. We really shouldn't even allow users to post links. They should be forbidden.

    The cleanvars here is just an example of what could be making the url link just be a navigation link to take us off the page:

    https://github.com/renlok/WeBid/blo...1210dd649dd6e/includes/functions_sell.php#L49

    I am taking a break for now. I will look at this later. If you see what it might be then share what you see or a fix here please.
     
  8. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    That works fine in the admin/editauction.php page. I can use the ckeditor there in the admin panel and just highlight a word and then select the link icon and set the target to New Window (_blank) and hit process changes and it will make the link so, it opens in a new window.

    Then I go to logged in user account and I look I view my auction item and I see I can edit it. I select Seller: Edit this item and when I click on the link in the description and I look at the target it's set with Window (_blank) .

    I believe it gets filtered out when I hit the submit auction on the sell.php page it processes it. It changes back the selected target from Window (_blank) back to <not set>.

    So, I believe the ckeditor is working properly. It's the final submit process that changed the target back to <not set>.
     
    Last edited: May 23, 2022
  9. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    I have figured out what was causing the issue of removing the extra stuff added using the target tab in the url link in the ckeditor description.

    It starts here in this section in the includes/function_global.php section around line 199.

    https://github.com/renlok/WeBid/blo...0dd649dd6e/includes/functions_global.php#L195

    PHP:
            if ($allow_html) {
                
    $config = array('safe' => 1'elements' => 'a, ol, ul, li, u, strong, em, br, p''deny_attribute' => '* -href');
            }
    The deny_attribute is connected to includes/packages/htmLawed.php at Line 40 where the str_replace replaces the target selection.

    Here's the link to the search on github:
    https://github.com/renlok/WeBid/search?q=deny_attribute

    I removed the last part of code in that line , 'deny_attribute' => '* -href' in the functions_global.php page like this:

    PHP:
            if ($allow_html) {
                
    $config = array('safe' => 1'elements' => 'a, ol, ul, li, u, strong, em, br, p');
    and it works now. It allows me to select the link target and the next time I go back to it, the target for the link is still selected on the one I selected.

    The question that I have now is, how safe is that?
     
    Last edited: May 26, 2022
  10. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    @timw255 this was you contribution to allow html. No denying that you are the best coder that I have ever seen at work on the Webid project. It's hard to believe it was 6 years ago that you were zipping through here giving Webid a breath of fresh air and a new life. You were incredible with coding here!

    You added the code so, it would allow html links. I have tested it with the previous code and it disabled the html link when I used the previous code and submitted my item. It was a good contribution but, needed a slight adjustment.
    https://github.com/renlok/WeBid/commit/c2d2542885ad7746ec95a4fdc1b2e3e3e7da2058

    Here's the comment on the change. allow hrefs (for links)
    https://github.com/renlok/WeBid/bla...a75896111b/includes/functions_global.php#L215

    This is how it was:
    PHP:
    $config = array('safe' => 1'elements' => 'a, ol, ul, li, u, strong, em, br, p''deny_attribute' => '* -href');
    dropping the minus (-) before the href works but, I am not sure why and that's what I am trying to figure out what's happening there and I want to make sure I keep the coding safe so, I could use a little assistance on this to make sure it remains safe.

    It works like this:
    PHP:
    $config = array('safe' => 1'elements' => 'a, ol, ul, li, u, strong, em, br, p''deny_attribute' => '* href');
    This allows the target selections on the url to be selected and submitted and not be set back to <not set>.

    To me, I believe the astric (*) is a wildcard which should mean any attribute added should be denied.

    The codes for the includes/packages/htmLawed.php page are a lot more difficult for me to trace what codes should be filtered or replaced. There is definitely a lot to go through there.
     
  11. david62311

    david62311 Well-Known Member

    Joined:
    Aug 29, 2013
    Messages:
    2,183
    Likes Received:
    251
    Don't worry, I am not working 24/7 on this...haha! I am just looking at it for a few minutes at a time.

    I found was I was looking for and now I understand what timw255 did. There was a security issue there and it's mentioned here.

    https://github.com/renlok/WeBid/blob/fe500f64b65294842f0c25ce1fd1210dd649dd6e/js/ckeditor/CHANGES.md

    timw255 didn't want to spell it out what he did there because he didn't want it to be obvious in his comment.

    What we don't see is when we post a link in the ckeditor is what is happening with the code. We have to right click and inspect the string of code we are putting there. It should look like this when it's allowing us to select the target to open in a new page:

    PHP:
    <a data-cke-saved-href="https://www.demo.edu/"
    target="_blank" href="https://www.demo.edu/">
    Phonamana YouTube Free Subscription</a>
    Notice the -data-cke-saved-href.: timw255 used the * -href to disable to the selection from being made when we use the url link and the target but, he still allowed us to post a link there using this:
    PHP:
    'deny_attribute' => '* -href'
    So, now we know there was a security issue ckeditor 4 and it's included in the script and combined with using ckeditor 5. In the admin panel there are no filters on editing an auction.

    Just a note: People should NOT log into public wi-fi anyways but, it does happen.

    This is what it reads in the changelog.

    • [Severity: minor] Fixed the target="_blank" vulnerability reported by James Gaskell.

      Issue summary: If a victim had access to a spoofed version of ckeditor.com via HTTP (e.g. due to DNS spoofing, using a hacked public network or mailicious hotspot), then when using a link to the ckeditor.com website it was possible for the attacker to change the current URL of the opening page, even if the opening page was protected with SSL.

      An upgrade is recommended.
    This has been fixed by timw255. Thanks!
     

Share This Page