TinyPortal

Development => Feedback => Bugs/Errors => Topic started by: Oldiesmann on June 06, 2020, 04:59:32 AM

Title: \ in action causes database error
Post by: Oldiesmann on June 06, 2020, 04:59:32 AM
Not sure this is worth fixing but I thought I'd report it anyway. Saw an error related to this in my forum's error log.


The query that handles finding blocks to show for a specific "action" doesn't escape the action, so a stray \ will cause an error.


Database Error: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '=allpages', access2))

AND (FIND_IN_SET(-1, access))
ORDER BY bar, pos,' at line 9




SELECT * FROM smf_tp_blocks
WHERE off = 0
AND bar != 4
AND (




FIND_IN_SET('actio=signup', access2) OR FIND_IN_SET('actio=signup\', access2) OR FIND_IN_SET('actio=allpages', access2))

AND (FIND_IN_SET(-1, access))
ORDER BY bar, pos, id ASC



This occurs at line 2223 of Sources/TPortal.php


Other than generating a bunch of errors (the DB error followed by 5 "Undefined index: tp_panels" and 5 "in_array() expects parameter 2 to be array, null given" errors), it doesn't cause any problems (SMF just shows a generic error message to the user if they're not an admin), and I can't think of any legitimate reason why you'd have a \ in the action anyway, but it might be a good idea to escape it just in case.
Title: Re: \ in action causes database error
Post by: tino on June 13, 2020, 05:32:14 PM
what version of TinyPortal are you using? I am struggling to recreate this atm.

I tried action=forum\f but it doesn't throw a error.
Title: Re: \ in action causes database error
Post by: Oldiesmann on June 14, 2020, 04:16:05 AM
I believe it only occurs when the \ is the last character in the action string. In my case the action that triggered it is "signup\". I'm using TP 1.6.6.
Title: Re: \ in action causes database error
Post by: lurkalot on June 14, 2020, 08:25:44 AM
Quote from: Oldiesmann on June 14, 2020, 04:16:05 AM
I believe it only occurs when the \ is the last character in the action string. In my case the action that triggered it is "signup\". I'm using TP 1.6.6.

Yep that triggers it for me too. I'm running TP 1.6.7

Title: Re: \ in action causes database error
Post by: tino on June 14, 2020, 08:32:14 AM
Thanks, I think it's fixed in Version 2, I'll move that fix to 1.6.x also.
Title: Re: \ in action causes database error
Post by: lurkalot on June 14, 2020, 08:36:37 AM
Quote from: tino on June 14, 2020, 08:32:14 AM
Thanks, I think it's fixed in Version 2, I'll move that fix to 1.6.x also.

Tino, thanks.  I noticed you get about a page full of errors in log in SMF 2.0 when you do this. In SMF 2.1 a different looking error on page when doing this, but about 50 errors in log. 

I'll go check TP 2.0.0
Title: Re: \ in action causes database error
Post by: lurkalot on June 14, 2020, 08:38:44 AM
Quote from: tino on June 14, 2020, 08:32:14 AM

Thanks, I think it's fixed in Version 2


Yep I don't have this issue in TP 2.0.0  O0
Title: Re: \ in action causes database error
Post by: @rjen on June 14, 2020, 09:08:54 AM
Unfortunately I do find a similar issue in 2.0.0 as well.

tried adding the backslash after a page number... like this
https://test.fjr-club.nl/index.php?cat=Nieuws\
On a test forum running SMF 2.0.17 and TP200.

bam:

Quote
You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''tpcat=Nieuws\', access2))
AND 1=1
ORDER BY bar, pos, id ASC' at line 4
Bestand: /home/deb77453/domains/fjr-club.nl/public_html/test/Sources/TPBlock.php
Regel: 163

doing the same in 167 with 2.0.17  does not give any errors, so this is a 2.0 issue
alone.
But it does not error on an SMF 2.1 test forum with TP 200
Title: Re: \ in action causes database error
Post by: lurkalot on June 14, 2020, 09:18:28 AM
Yep.  You're right it does.  ;)

Quote
Database Error
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''tpage=ebay-rss\', access2) OR FIND_IN_SET('', access2))
AND 1=1
ORDER BY' at line 4
File: /home/vol11_7/byethost7.com/b7_24299229/htdocs/testsite5/Sources/TPBlock.php
Line: 163
Title: Re: \ in action causes database error
Post by: lurkalot on June 14, 2020, 09:27:05 AM
Same thing happening in downloads as well in TP 1.6.7

action=tpmod;dl\
action=tpmod;dl=item14\

etc

I noticed in TP 2.0.0 though, action=tpmod;dl\ give a,

"An Error Has Occurred!
Unable to load the 'main' template."
Title: Re: \ in action causes database error
Post by: tino on June 14, 2020, 11:17:05 AM
That doesn't surprise me. There is loads of unescaped data strings.

I'll see if I can parse it all at the start and clear them out in one place. Although than may cause other issues.
Title: Re: \ in action causes database error
Post by: tino on June 14, 2020, 04:31:54 PM
I'm not sure what's the best way forward with this, it is in so many places and stripping them all out breaks other things in SMF. So it's either do it properly ( like it should of been in the first place ) or leave it as is. I can't seem to exploit it mind.
Title: Re: \ in action causes database error
Post by: @rjen on June 14, 2020, 04:37:35 PM
It seems to have been like this for quite some time. TBH I would not bother fixing it in 1.6.x branch. In 2.0.0 it would be nice to fix this where possible, but I cannot fully appreciate the impact that will have on other things...
Title: Re: \ in action causes database error
Post by: tino on June 14, 2020, 07:21:35 PM
Quote from: @rjen on June 14, 2020, 04:37:35 PM
It seems to have been like this for quite some time. TBH I would not bother fixing it in 1.6.x branch. In 2.0.0 it would be nice to fix this where possible, but I cannot fully appreciate the impact that will have on other things...

It's pretty much everywhere a _GET or _POST request is used and not escaped properly. Which looking at it is most of the code. When the built in SMF string checks are used it's ok, it's not when the string is manually constructed, so the $access with FIND_IN_SET is a good example. I don't like that anyway, just couldn't find an easy alternative at the time.
Title: Re: \ in action causes database error
Post by: Oldiesmann on June 16, 2020, 03:18:57 AM
I need to play with TP 2.0 at some point... Maybe that would be a good thing to use my newly-purchased ".rocks" domain for :P
Title: Re: \ in action causes database error
Post by: @rjen on June 16, 2020, 06:56:32 AM
Quote from: Oldiesmann on June 16, 2020, 03:18:57 AM
I need to play with TP 2.0 at some point... Maybe that would be a good thing to use my newly-purchased ".rocks" domain for :P

TP 2.0.0 is pretty much ready, we could really use someone else's input on it before it can be released, so yes please!
Title: Re: \ in action causes database error
Post by: tino on June 16, 2020, 08:12:59 PM
Quote from: @rjen on June 14, 2020, 09:08:54 AM
Unfortunately I do find a similar issue in 2.0.0 as well.

tried adding the backslash after a page number... like this
https://test.fjr-club.nl/index.php?cat=Nieuws\
On a test forum running SMF 2.0.17 and TP200.

bam:

Quote
You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ''tpcat=Nieuws\', access2))
AND 1=1
ORDER BY bar, pos, id ASC' at line 4
Bestand: /home/deb77453/domains/fjr-club.nl/public_html/test/Sources/TPBlock.php
Regel: 163

doing the same in 167 with 2.0.17  does not give any errors, so this is a 2.0 issue
alone.
But it does not error on an SMF 2.1 test forum with TP 200

Fixed this one in my last commit and removed some duplicate code whilst I was at it.
Title: Re: \ in action causes database error
Post by: @rjen on June 16, 2020, 10:37:28 PM
Checked and  O0
Title: Re: \ in action causes database error
Post by: Oldiesmann on June 17, 2020, 03:51:38 AM
Quote from: @rjen on June 16, 2020, 06:56:32 AM
Quote from: Oldiesmann on June 16, 2020, 03:18:57 AM
I need to play with TP 2.0 at some point... Maybe that would be a good thing to use my newly-purchased ".rocks" domain for :P

TP 2.0.0 is pretty much ready, we could really use someone else's input on it before it can be released, so yes please!

Just installed it at www.oldiesmann.rocks. Will play around with it some and let you know what I think.

I can also test it with PostgreSQL as well if you'd like.
Title: Re: \ in action causes database error
Post by: tino on June 17, 2020, 05:30:17 PM
Most of my development is done in PostgreSQL, about 90% of it as I prefer it to MySQL, but another person testing it would be good. Especially as I've only really used 9.4 or above.
Title: Re: \ in action causes database error
Post by: Oldiesmann on June 18, 2020, 06:08:31 AM
I don't remember what version I have on my server. I think it's 10.something but I haven't updated it in ages.

Also, what all has changed with 2.0.0? Comparing it with a 1.6.6 install the only big thing I see is that the Yes/No radio button pairs have been replaced with checkboxes
Title: Re: \ in action causes database error
Post by: @rjen on June 18, 2020, 06:17:21 AM
Most of it is 'under the hood'  changes an not new functionality... below is draft but pretty much the complete change log...

QuoteBelow the changes made to the branch labeled 2.0.0

This includes the following updates and fixes:
Relevant to 2.0 and 2.1:

Technical updates:
- major updates to codebase, cleaned up sources, templates and language files
- changes to database tables and fields
- removed the unused code sections for plugins and modules
- removed non-functional permission 'always approved'
- fixed a number of hard-coded language strings
- updated language string usage in for easier translation

New functionality:
- added setting to Disable the PHP eval function for templates
- added configurable paths for TinyPortal images, downloads and blockcodes
- added setting that allows the removal of Tinyportal menu option for all users but admins
- added setting to allow / disallow links in comments
- updated article comments display and added comments counters
- replaced 5-boards option with multi-select for news boards topics frontpage
- added 'include or exclude boards' option to recent topics block
- updated block sorting function to resolve issues when moving blocks up and down
- added extra css option for custom css
- allow for over 30 search results and pagination on article search
- replaced general settings with checkboxes instead of radio buttons
- updated TinyPortal menu option allowing direct access to settings pages

Relevant to 2.1 only:
- added Mentions to Shoutbox and article comments for SMF2.1

NOTICE!  because of the database changes in this version there's no going back to TP 1.x.x after installing version 2.0.0. You can install previous versions, but by doing that you WILL lose TP data and you will need to manually fix or recreate your content!!!
Title: Re: \ in action causes database error
Post by: lurkalot on June 18, 2020, 07:43:19 AM
Also Re:  New functionality:

The ability to add external downloads, via link.
Title: Re: \ in action causes database error
Post by: tino on June 18, 2020, 09:32:03 AM
It was never meant to be massive changes to the user, just a rewrite of the logic to make it more friendly and follow more modern design practices.

It's kind of done that, but require a fair bit more work. I'd like to switch to using flex based css for the layout but tbh that's not my realm of expertise. 
Title: Re: \ in action causes database error
Post by: Oldiesmann on July 03, 2020, 03:00:15 AM
Moved my Archie site over to TP 2.0 last night (from EhPortal). Had to tweak my shoutbox importing code to account for the change in column names but other than that it seems to be working well.

I did notice a couple of issues though...

1. With layout 3, the shoutbox displays the date/time before the username in addition to on the far right side of the shoutbox. It should only display on the far right side.

2. What happened to the option to enable/disable modules? It's not a huge deal since I can use permissions to hide things I don't use, but it just seems odd that it's not there anymore.
Title: Re: \ in action causes database error
Post by: Oldiesmann on July 03, 2020, 05:52:35 AM
One other thing, which seems to be a recent development (comparing archiefans.com and oldiesmann.rocks)...

The blocks in the left and right panels now appear inside another "panel" of some sort rather than standing out on their own. This looks weird and also reduces the width of the blocks.
Title: Re: \ in action causes database error
Post by: @rjen on July 03, 2020, 06:13:00 AM
Can I see what you are referring to somewhere?

What SMF version are you using?

What theme?
Title: Re: \ in action causes database error
Post by: @rjen on July 03, 2020, 06:49:01 AM
found it...
https://www.archiefans.com/

As for the right and left panel: it seems you have set to use the roundframe for them: check Tinyportal > Settings & Frontpage: Use the roundframe style for left/right panels

(that setting is pretty old by the way and I doubt it's usefulness, so I was considering to remove it...)

The modules enable / disable has been removed since the whole modules concept has been removed: that has never really brought what it was intended to bring. So the 'modules' TPdownloads, TPshout and TPlistimages are now just an integral part of TinyPortal.

Instead there is a switch in the TPdownloads settings now that allows you to switch it off:  TinyPortal > Manage TPdownloads > Settings: "Show Downloads"

I'll check the shoutbox layout: that's new to me..


Title: Re: \ in action causes database error
Post by: @rjen on July 03, 2020, 07:05:06 AM
Found the layout bug... fixed now. You can use attached file if you like...
Title: Re: \ in action causes database error
Post by: Oldiesmann on July 03, 2020, 07:17:55 AM
Thanks for the quick response. Not sure how I missed that roundframe option.

Updated my copy of TPShout.template.php as well via the admin center (faster than downloading a new copy and uploading it again)
Title: Re: \ in action causes database error
Post by: @rjen on July 03, 2020, 07:19:35 AM
Cool, let us know if you find anything else that needs to be fixed / improved...
Title: Re: \ in action causes database error
Post by: lurkalot on July 03, 2020, 07:41:17 AM
Oldiesmann, thanks for helping out on testing, you've been a great help already. Always good to have some more eyes on it, and setting it up in different ways etc.   

archiefans is looking good, which reminds me, I must get around to finishing that theme at some point, think I missed most of the admin area. :-[

@rjen thanks for jumping in and sorting these issues so quickly, nice job.  O0