volosoft / jtable

A JQuery plugin to create AJAX based CRUD tables.
http://www.jtable.org
1.1k stars 506 forks source link

A # character in the fieldName of column header breaks sorting (with simple potential fix) #2141

Open Andrewp800x opened 6 years ago

Andrewp800x commented 6 years ago

If there is a # character in a column header, the sorting breaks as the its fieldName has not been encoded for a URI. I have fixed the problem by using encodeURIComponent on $columnHeader.data('fieldName') in lines 4356 and 4362 of version 2.5.0 currently.

//Sort ASC or DESC according to current sorting state if ($columnHeader.hasClass('jtable-column-header-sorted-asc')) { $columnHeader.removeClass('jtable-column-header-sorted-asc').addClass('jtable-column-header-sorted-desc'); this._lastSorting.push({ 'fieldName': encodeURIComponent($columnHeader.data('fieldName')), sortOrder: 'DESC' }); } else { $columnHeader.removeClass('jtable-column-header-sorted-desc').addClass('jtable-column-header-sorted-asc'); this._lastSorting.push({ 'fieldName': encodeURIComponent($columnHeader.data('fieldName')), sortOrder: 'ASC' }); }

I know that I should create a pull request, but I would appreciate input from others first on this small fix. Thanks!

Andrewp800x commented 6 years ago

In dev, the issues are found on lines 157 and 163 of jquery.jtable.sorting.js in the _sortTableByColumn function.

sheryever commented 6 years ago

Why you need the # character, in database you cannot have the # character as name of the column. If you want to show the # character in the header title then use the title option of the field.

fieldName is a javascript identifier of your field options object

Andrewp800x commented 6 years ago

I want and can have, through proper escaping, the same name for everything no matter where it is (except for maybe my database due to character count restrictions that I have workarounds for). :100:

Andrewp800x commented 6 years ago

I follow KISS ("Keep it simple, stupid") when I can.

sheryever commented 6 years ago

Sorry, I have no answer for that 🤣. Well, i will suggest you to don't do this simple thing with jtable and just keep your field name without # character and when you receive the field name on server in sortby parameter, use if condition and create your own sortby column name with # character and just Keep it simple.

Andrewp800x commented 6 years ago

I was surprised that a POST wasn't used for sorting.

Andrewp800x commented 6 years ago

As a side note, I noticed that sorting is a weak spot in terms of injection if developers aren't careful as everything is just strung together rather than using an object.

sheryever commented 6 years ago

jtable or any other jquery plugin will never secure you from sql injection while ajax as all the request goes through Http/Https( if any case https also compromise), it must be handled at the level of executing the queries.

Andrewp800x commented 6 years ago

I like to abstract things away from one off branching logic that wouldn't cover the whole character set is all. I'm just saying that an object would be nicer than using Regex fu that not everyone has is all.

Andrewp800x commented 6 years ago

I am escaping everything as everybody should. You are right.

Andrewp800x commented 6 years ago

I read the news too much.

sheryever commented 6 years ago

Reading news is not any issue, just don't let it drive you

Andrewp800x commented 6 years ago

I'm just waiting for a client to need a column with ASC, at the end of its name...

Andrewp800x commented 6 years ago

I know your fieldName point...

sheryever commented 6 years ago

ASC are you talking about SomeClumnName ASC in jtSorting get parameter?

Andrewp800x commented 6 years ago

Not quite. More like "SomeColumnName ASC," so the sort would read SomeColumnName ASC, ASC,SomeOtherColumnName ASC. My Regex is greedy enough though I believe.

sheryever commented 6 years ago

Ok, we are sending the default ORDER BY, and we use it as it is with replacing the ' with '' to avoid the sql injection like

 var sql = "SELECT Name, Address, Phone FROM dbo.Contacts ORDER BY " + jtSorting.Replace("'", "''");
 // query execution 

What server side language and data access you are using?

Andrewp800x commented 6 years ago

PHP MariaDB right now I have protected columns in some tables so I went nuts and atomized with Regex, checked column names against user roles, escaped them, and made an engine that makes prepared statements for lack of a better term as I have a query builder as well.

Andrewp800x commented 6 years ago

I'm using some custom mysqli stuff too if that's what you were asking.

sheryever commented 6 years ago

Well, if you are using query builder then you really need to parse your jtSorting parameter.

Andrewp800x commented 6 years ago

It seems only natural. I'd love to see an integration like mine that's open source.

Andrewp800x commented 6 years ago

I don't know why applications are pushing stuff that's shady.

sheryever commented 6 years ago

jtable is a popular jquery plugin, there must be some php parser for this kind of work, which will generate a dictionary for you. as i think.

sheryever commented 6 years ago

It's not jtable only, almost all table plugins in javascript send the data on server in the same way, now it's on the developer how they process the data on server.

Andrewp800x commented 6 years ago

Right. There's jQuery-QueryBuilder and QBJSParser, but they only go so far right now.

Andrewp800x commented 6 years ago

You still need a ORM/ODM that isn't annoying.

Andrewp800x commented 6 years ago

And rules for your rules...

sheryever commented 6 years ago

I use mine SimpleAccess-orm and I use stored procedure for paged data.

Andrewp800x commented 6 years ago

Nice

Andrewp800x commented 6 years ago

I wish others were as competent...

sheryever commented 6 years ago

Well, I can support MariaDB in C# but for php... a whole different world.

Andrewp800x commented 6 years ago

Let me know when you can get others to understand the importance of simple queries / lists and UUIDs that have anything to do with the GUI or even half of what CRUDL is when writing an API.

Andrewp800x commented 6 years ago

PSA FYI sheryever's code, as follows, is quite vulnerable

Please do not use it ever and be sure to always properly sanitize all your inputs including the order by clause. Escaping single quotes is not sufficient.

var sql = "SELECT Name, Address, Phone FROM dbo.Contacts ORDER BY " + jtSorting.Replace("'", "''"); // query execution

Examples: https://support.portswigger.net/customer/portal/articles/2590771-Methodology_SQLi-QueryStructure.html http://securityidiots.com/Web-Pentest/SQL-Injection/group-by-and-order-by-sql-injection.html#xpath https://www.notsosecure.com/injection-in-order-by-clause/ etc.

https://www.owasp.org/index.php/SQL_Injection is a great place to start

sheryever commented 6 years ago

Thanks @Andrewp800x

It is a good PSA, because ORDER By is really vulnerable.

I write my query in a way that Order by vulnerability will not hit me and throw the error but is there any solution to this vulnerability because i was searching the solution for other and my own information and found there are many ways to exploit this vulnerability.

The two solution which i discover for now are: First check the jtSorting (or parameters like that) with switch / if in your code and set the value of order by by your self. Second use the regular expression and parse the column name by your self