New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xss major vulnerabilities #331

Closed
alindima opened this Issue Oct 13, 2015 · 18 comments

Comments

Projects
None yet
6 participants
@alindima
Copy link

alindima commented Oct 13, 2015

Hello everyone.
Grocery crud is extremely vulnerable to xss attacks..The output that is being displayed in a regular table is not escaped at all,and as we all know, that is a massive problem.
Please fix this ASAP.
Thanks.

@airways

This comment has been minimized.

Copy link

airways commented Oct 13, 2015

This is extremely irresponsible disclosure. Way to go.

@alindima

This comment has been minimized.

Copy link

alindima commented Oct 13, 2015

@airways ,sorry but i did not get your point

@airways

This comment has been minimized.

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Oct 13, 2015

Hello @alindima,

I am sorry to admit it but @airways is right. Can you please send me at my personal email a more specific issue of what the problem exactly is and suggestions to actually solve the issue?

Thanks
Johnny

@heruprambadi

This comment has been minimized.

Copy link

heruprambadi commented Oct 20, 2015

any solution how to prevent that ?

@buoncri

This comment has been minimized.

Copy link
Contributor

buoncri commented Oct 21, 2015

Using GC only in Admin area :-D

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Oct 22, 2015

I did start a new branch to fix that issue. If you urgently need it then you can have a hot fix of just filter all the inputs from here: cb82d94 of course this branch it is not completed yet as it needs to have a config file and some other validation checks. I just published this change in case someone needs that urgently.

Thanks
Johnny

@agung-wete

This comment has been minimized.

Copy link

agung-wete commented Oct 23, 2015

As suggested by codeigniter user guide :

rule about XSS cleaning is that it should only be applied to output, as opposed to input data.

We’ve made that mistake ourselves with our automatic and global XSS cleaning feature (see previous step about XSS above), so now in an effort to discourage that practice, we’re also removing ‘xss_clean’ from the officially supported list of form validation rules.

Sometimes, we need that "special" character that filtered by execute xss_clean on input (for example: password field)

Because this is CRUD library, we cant determine which input need this, so we shouldnt do xss_clean globally.

To prevent XSS we need to do html_escape in output field (function change_list, get_add_input_fields, get_edit_input_fields, get_read_input_fields). This is also make sure ' " ' from data dont break input tag.

Suggestion :

  • Because for now grocery crud depends on codeigniter, we need not reinventing the wheel 😃 .
    use codeigniter html_escape or xss_clean function
  • If we really need to do xss cleaning in input data, we should able to choose which field NOT filtered.
    maybe add some function : no_xss_clean_input($arrary_of_field_name)
    If it in $arrary_of_field_name then we dont do xss_clean on it

Thank you

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Oct 24, 2015

@agung-wete thanks for that and to be honest I like that this is now a thread for brainstorming :)

I actually agree with what Codeigniter is saying to not have it globally. That's why the fix is half way through and it is not completed yet.

@agung-wete first of all although grocery CRUD is depended on Codeigniter the library is built with that way so one day will NOT be Codeigniter depended at the future. So currently the only libraries that grocery CRUD is using from CI are:

  • form validation
  • database
  • url helper

Nothing more than that (not even the session or the layout is not in use)

Second xss_clean of Codeigniter is really really slow (especially for big texts). That's the main reason that I don't want to use it and I am trying to find another solution.

I will update you for any changes that I will make to the code. The code is not yet completed yet and new ideas are more than welcome :)

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Sep 28, 2016

New upcoming version 1.5.7 with XSS clean. If you are interested you can download the master branch. Enjoy :)

@heruprambadi

This comment has been minimized.

Copy link

heruprambadi commented Sep 28, 2016

hurray ! 💃 💃

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Sep 28, 2016

The version is now in public with plus an optimisation of the totals on big tables

@heruprambadi

This comment has been minimized.

Copy link

heruprambadi commented Jan 8, 2017

Where should i find documentation about it ?

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Jan 8, 2017

There is not currently any documentation. It is as easy as setting the config file grocery_crud_xss_clean into true.

More specifically the file is at: application/config/grocery_crud.php

The main reason I did that is that in case you will need to skip the xss_clean validation only for 1 of your CRUD, you can simply do this:

 $this->load->config('grocery_crud');
 $this->config->set_item('grocery_crud_xss_clean', false);

before calling the grocery CRUD.

Also have in mind (I am actually copying the comments at the config) that:

// Turn XSS clean into true in case you are exposing your CRUD into public. 
// Please be aware that this is stripping all the HTML and do not just trim the extra javascript
@heruprambadi

This comment has been minimized.

Copy link

heruprambadi commented Jan 8, 2017

Oh, ok. I just need to know how to use it. Thanks 😆
Anyway, where (again) i should find details about change log ? Like how to use "faster way to calculate total" (gc 1.5.7).

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Jan 8, 2017

I am trying to keep track at the github. So for example if you see a number like (#370: A faster way to calculate the totals) The #370 is a link that you can click and see all the discussions/commits/pull requests and history of the enhancement.

@scoumbourdis

This comment has been minimized.

Copy link
Owner

scoumbourdis commented Jan 8, 2017

You can basically see the change logs (with the urls) here: http://www.grocerycrud.com/documentation/change_logs

@heruprambadi

This comment has been minimized.

Copy link

heruprambadi commented Jan 8, 2017

So there is change log on your site.. ok thanks again 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment