Skip to content
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

Url quoting/unquoting for keywords #22

Open
skyl opened this issue Nov 8, 2015 · 4 comments
Open

Url quoting/unquoting for keywords #22

skyl opened this issue Nov 8, 2015 · 4 comments

Comments

@skyl
Copy link

skyl commented Nov 8, 2015

I tried to make a keyword, "alzheimer's". When I click on the tag, alzheimer's, from the list page, I am directed to /publications/tag/alzheimer%2527s/ which has no results. Something is wrong with how the keywords are quoted/unquoted.

I looked in the code a bit and saw that keywords are represented as a CharField. I was hoping to see a FK into a tagging library dependency. I haven't look much farther. At any rate, I think the quoting/unquoting should reliably reverse each other.

@skyl
Copy link
Author

skyl commented Nov 8, 2015

One issue is that it's double escaping with the url tag:

    {% for keyword, keyword_escaped in publication.keywords_escaped %}
    <a class="keyword" href="{% url 'publications.views.keyword' keyword_escaped %}">{{ keyword }}</a>
    {% endfor %}

@lucastheis
Copy link
Owner

You are right, I should use a tagging library. A switch to django-tagging is planned, but unfortunately I don't have enough time at the moment to actually implement that change.

@skyl
Copy link
Author

skyl commented Nov 10, 2015

I'll see what I can do. It looks like django-taggit is more popular these days. I'm going to see if I can do it with migrations and tests and fix this issue along the way.

I'm using Django 1.8.6 and Python 3.4.2. I'm not sure how backwards compatible this is going to be as a first take. I'm going to use all of the newest Django migrations stuff.

skyl added a commit to skyl/django-publications that referenced this issue Nov 10, 2015
@orlra
Copy link

orlra commented Feb 27, 2017

File "/data/www/venv/local/lib/python2.7/site-packages/publications/views/person.py", line 53, in person
for publication in query:
ProgrammingError: syntax error at or near "whs"
LINE 1: ...s_publication.authors) LIKE lower('%none/CONCAT('whs(',')SQ...

this is fragment of error I received today.
path:/publications/s.+none/CONCAT('whs(',')SQLi')/,

which is basically UN-escaped string send to database.
I'm still investigating, but it looks like its possible to sql inject.
Django 1.8.17
django-publications 0.6.2/0.6.1/develop branch.

the diff may come from this:
author is not escaped properly

        def keywords_escaped(self):
                return [(keyword.strip(), urlquote_plus(keyword.strip()))
                        for keyword in self.keywords.split(',')]


        def authors_escaped(self):
                return [(author, author.lower().replace(' ', '+'))
                        for author in self.authors_list]


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants