engineering/unrhodecode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

security: use new safe escaped user attributes across the application.

Browse source 
- will fix all possible XSS attack vectors.
This commit is contained in:
Marcin Lulek 2017-06-19 15:14:02 +02:00
parent 91e65dd798
commit ad54b9a57d
17 changed files with 50 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

4
rhodecode/apps/admin/views/users.py
View file

@ -127,8 +127,8 @@ class AdminUsersView(BaseAppView, DataGridAppView):
users_data.append({
"username": h.gravatar_with_user(user.username),
"email": user.email,
"first_name": h.escape(user.name),
"last_name": h.escape(user.lastname),
"first_name": user.first_name,
"last_name": user.last_name,
"last_login": h.format_date(user.last_login),
"last_activity": h.format_date(user.last_activity),
"active": h.bool2icon(user.active),

4
rhodecode/apps/home/tests/test_home.py
View file

@ -111,8 +111,8 @@ class TestHomeController(TestController):
user_util.create_repo(owner=username)
response = self.app.get(route_path('home'))
response.mustcontain(h.html_escape(h.escape(user.name)))
response.mustcontain(h.html_escape(h.escape(user.lastname)))
response.mustcontain(h.html_escape(user.first_name))
response.mustcontain(h.html_escape(user.last_name))
@pytest.mark.parametrize("name, state", [
('Disabled', False),

4
rhodecode/apps/repository/utils.py
View file

@ -36,8 +36,8 @@ def reviewer_as_json(user, reasons=None, mandatory=False):
'reasons': reasons or [],
'mandatory': mandatory,
'username': user.username,
'firstname': user.firstname,
'lastname': user.lastname,
'first_name': user.first_name,
'last_name': user.last_name,
'gravatar_link': h.gravatar_url(user.email, 14),
}

4
rhodecode/controllers/admin/user_groups.py
View file

@ -493,8 +493,8 @@ class UserGroupsController(BaseController):
group_members = [
{
'id': user.user_id,
'first_name': user.name,
'last_name': user.lastname,
'first_name': user.first_name,
'last_name': user.last_name,
'username': user.username,
'icon_link': h.gravatar_url(user.email, 30),
'value_display': h.person(user.email),

3
rhodecode/controllers/pullrequests.py
View file

@ -21,8 +21,6 @@
"""
pull requests controller for rhodecode for initializing pull requests
"""
import types
import peppercorn
import formencode
import logging
@ -33,6 +31,7 @@ from pylons import request, tmpl_context as c, url
from pylons.controllers.util import redirect
from pylons.i18n.translation import _
from pyramid.threadlocal import get_current_registry
from pyramid.httpexceptions import HTTPFound
from sqlalchemy.sql import func
from sqlalchemy.sql.expression import or_

2
rhodecode/lib/auth.py
View file

@ -807,6 +807,8 @@ class AuthUser(object):
self.ip_addr = ip_addr
self.name = ''
self.lastname = ''
self.first_name = ''
self.last_name = ''
self.email = ''
self.is_authenticated = False
self.admin = False

4
rhodecode/lib/channelstream.py
View file

@ -77,8 +77,8 @@ def get_user_data(user_id):
return {
'id': user.user_id,
'username': user.username,
'first_name': user.name,
'last_name': user.lastname,
'first_name': user.first_name,
'last_name': user.last_name,
'icon_link': h.gravatar_url(user.email, 60),
'display_name': h.person(user, 'username_or_name_or_email'),
'display_link': h.link_to_user(user),

8
rhodecode/lib/helpers.py
View file

@ -893,9 +893,9 @@ def author_string(email):
if email:
user = User.get_by_email(email, case_insensitive=True, cache=True)
if user:
if user.firstname or user.lastname:
if user.first_name or user.last_name:
return '%s %s <%s>' % (
escape(user.firstname), escape(user.lastname), email)
user.first_name, user.last_name, email)
else:
return email
else:
@ -1144,14 +1144,14 @@ class InitialsGravatar(object):
# first push the email initials
prefix, server = email_address.split('@', 1)
# check if prefix is maybe a 'firstname.lastname' syntax
# check if prefix is maybe a 'first_name.last_name' syntax
_dot_split = prefix.rsplit('.', 1)
if len(_dot_split) == 2:
initials = [_dot_split[0][0], _dot_split[1][0]]
else:
initials = [prefix[0], server[0]]
# then try to replace either firtname or lastname
# then try to replace either first_name or last_name
fn_letter = (first_name or " ")[0].strip()
ln_letter = (last_name.split(' ', 1)[-1] or " ")[0].strip()

20
rhodecode/model/db.py
View file

@ -574,12 +574,16 @@ class User(Base, BaseModel):
@hybrid_property
def first_name(self):
from rhodecode.lib import helpers as h
return h.escape(self.name)
if self.name:
return h.escape(self.name)
return self.name
@hybrid_property
def last_name(self):
from rhodecode.lib import helpers as h
return h.escape(self.lastname)
if self.lastname:
return h.escape(self.lastname)
return self.lastname
@hybrid_property
def api_key(self):
@ -700,7 +704,7 @@ class User(Base, BaseModel):
@property
def username_and_name(self):
return '%s (%s %s)' % (self.username, self.firstname, self.lastname)
return '%s (%s %s)' % (self.username, self.first_name, self.last_name)
@property
def username_or_name_or_email(self):
@ -709,20 +713,20 @@ class User(Base, BaseModel):
@property
def full_name(self):
return '%s %s' % (self.firstname, self.lastname)
return '%s %s' % (self.first_name, self.last_name)
@property
def full_name_or_username(self):
return ('%s %s' % (self.firstname, self.lastname)
if (self.firstname and self.lastname) else self.username)
return ('%s %s' % (self.first_name, self.last_name)
if (self.first_name and self.last_name) else self.username)
@property
def full_contact(self):
return '%s %s <%s>' % (self.firstname, self.lastname, self.email)
return '%s %s <%s>' % (self.first_name, self.last_name, self.email)
@property
def short_contact(self):
return '%s %s' % (self.firstname, self.lastname)
return '%s %s' % (self.first_name, self.last_name)
@property
def is_admin(self):

4
rhodecode/model/pull_request.py
View file

@ -1291,8 +1291,8 @@ class PullRequestModel(BaseModel):
'user': {
'user_id': repo.user.user_id,
'username': repo.user.username,
'firstname': repo.user.firstname,
'lastname': repo.user.lastname,
'firstname': repo.user.first_name,
'lastname': repo.user.last_name,
'gravatar_link': h.gravatar_url(repo.user.email, 14),
},
'description': h.chop_at_smart(repo.description, '\n'),

9
rhodecode/model/user.py
View file

@ -70,8 +70,8 @@ class UserModel(BaseModel):
return {
'id': user.user_id,
'first_name': h.escape(user.name),
'last_name': h.escape(user.lastname),
'first_name': user.first_name,
'last_name': user.last_name,
'username': user.username,
'email': user.email,
'icon_link': h.gravatar_url(user.email, 30),
@ -679,6 +679,11 @@ class UserModel(BaseModel):
# TODO: johbo: Think about this and find a clean solution
user_data = dbuser.get_dict()
user_data.update(dbuser.get_api_data(include_secrets=True))
user_data.update({
# set explicit the safe escaped values
'first_name': dbuser.first_name,
'last_name': dbuser.last_name,
})
for k, v in user_data.iteritems():
# properties of auth user we dont update

4
rhodecode/public/js/src/rhodecode/pullrequests.js
View file

@ -227,8 +227,8 @@ ReviewersController = function () {
for (var i = 0; i < data.reviewers.length; i++) {
var reviewer = data.reviewers[i];
self.addReviewMember(
reviewer.user_id, reviewer.firstname,
reviewer.lastname, reviewer.username,
reviewer.user_id, reviewer.first_name,
reviewer.last_name, reviewer.username,
reviewer.gravatar_link, reviewer.reasons,
reviewer.mandatory);
}

4
rhodecode/templates/admin/my_account/my_account_profile.mako
View file

@ -32,7 +32,7 @@
${_('First Name')}:
</div>
<div class="right-content">
${c.user.firstname}
${c.user.first_name}
</div>
</div>
<div class="fieldset">
@ -40,7 +40,7 @@
${_('Last Name')}:
</div>
<div class="right-content">
${c.user.lastname}
${c.user.last_name}
</div>
</div>
<div class="fieldset">

4
rhodecode/templates/base/root.mako
View file

@ -12,8 +12,8 @@ if getattr(c, 'rhodecode_user', None) and c.rhodecode_user.user_id:
c.template_context['rhodecode_user']['username'] = c.rhodecode_user.username
c.template_context['rhodecode_user']['email'] = c.rhodecode_user.email
c.template_context['rhodecode_user']['notification_status'] = c.rhodecode_user.get_instance().user_data.get('notification_status', True)
c.template_context['rhodecode_user']['first_name'] = c.rhodecode_user.name
c.template_context['rhodecode_user']['last_name'] = c.rhodecode_user.lastname
c.template_context['rhodecode_user']['first_name'] = c.rhodecode_user.first_name
c.template_context['rhodecode_user']['last_name'] = c.rhodecode_user.last_name
c.template_context['visual']['default_renderer'] = h.get_visual_attr(c, 'default_renderer')
c.template_context['default_user'] = {

4
rhodecode/templates/email_templates/user_registration.mako
View file

@ -10,7 +10,7 @@ RhodeCode new user registration: ${user.username}
A new user `${user.username}` has registered on ${h.format_date(date)}
- Username: ${user.username}
- Full Name: ${user.firstname} ${user.lastname}
- Full Name: ${user.first_name} ${user.last_name}
- Email: ${user.email}
- Profile link: ${h.route_url('user_profile', username=user.username)}
@ -21,7 +21,7 @@ ${self.plaintext_footer()}
<table style="text-align:left;vertical-align:middle;">
<tr><td colspan="2" style="width:100%;padding-bottom:15px;border-bottom:1px solid #dbd9da;"><h4><a href="${h.route_url('user_profile', username=user.username)}" style="color:#427cc9;text-decoration:none;cursor:pointer">${_('New user %(user)s has registered on %(date)s') % {'user': user.username, 'date': h.format_date(date)}}</a></h4></td></tr>
<tr><td style="padding-right:20px;padding-top:20px;">${_('Username')}</td><td style="line-height:1;padding-top:20px;"><img style="margin-bottom:-5px;text-align:left;border:1px solid #dbd9da" src="${h.gravatar_url(user.email, 16)}" height="16" width="16">&nbsp;${user.username}</td></tr>
<tr><td style="padding-right:20px;">${_('Full Name')}</td><td>${user.firstname} ${user.lastname}</td></tr>
<tr><td style="padding-right:20px;">${_('Full Name')}</td><td>${user.first_name} ${user.last_name}</td></tr>
<tr><td style="padding-right:20px;">${_('Email')}</td><td>${user.email}</td></tr>
<tr><td style="padding-right:20px;">${_('Profile')}</td><td><a href="${h.route_url('user_profile', username=user.username)}">${h.route_url('user_profile', username=user.username)}</a></td></tr>
</table>

4
rhodecode/templates/users/user_profile.mako
View file

@ -35,7 +35,7 @@
${_('First name')}:
</div>
<div class="right-content">
${c.user.firstname}
${c.user.first_name}
</div>
</div>
<div class="fieldset">
@ -43,7 +43,7 @@
${_('Last name')}:
</div>
<div class="right-content">
${c.user.lastname}
${c.user.last_name}
</div>
</div>
<div class="fieldset">

7
rhodecode/tests/functional/test_admin_gists.py
View file

@ -336,9 +336,7 @@ class TestGistsController(TestController):
def test_user_first_name_is_escaped(self, user_util, create_gist):
xss_atack_string = '"><script>alert(\'First Name\')</script>'
xss_escaped_string = (
'&#34;&gt;&lt;script&gt;alert(&#39;First Name&#39;)&lt;/script'
'&gt;')
xss_escaped_string = h.html_escape(h.escape(xss_atack_string))
password = 'test'
user = user_util.create_user(
firstname=xss_atack_string, password=password)
@ -348,8 +346,7 @@ class TestGistsController(TestController):
def test_user_last_name_is_escaped(self, user_util, create_gist):
xss_atack_string = '"><script>alert(\'Last Name\')</script>'
xss_escaped_string = (
'&#34;&gt;&lt;script&gt;alert(&#39;Last Name&#39;)&lt;/script&gt;')
xss_escaped_string = h.html_escape(h.escape(xss_atack_string))
password = 'test'
user = user_util.create_user(
lastname=xss_atack_string, password=password)