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

XSS mitigation by sanitizing user inputs with bleach #642

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions owasp-top10-2021-apps/a3/copy-n-paste/app/util/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func AuthenticateUser(user string, pass string) (bool, error) {
}
defer dbConn.Close()

query := fmt.Sprint("select * from Users where username = '" + user + "'")
rows, err := dbConn.Query(query)
query := ("SELECT * FROM Users WHERE username = ?")
rows, err := dbConn.Query(query, user)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -88,12 +88,12 @@ func NewUser(user string, pass string, passcheck string) (bool, error) {
}
defer dbConn.Close()

query := fmt.Sprint("insert into Users (username, password) values ('" + user + "', '" + passHash + "')")
rows, err := dbConn.Query(query)
query := ("INSERT INTO Users (username, password) VALUES (?, ?)")
rows, err := dbConn.Exec(query, user, passHash)
if err != nil {
return false, err
}
defer rows.Close()


fmt.Println("User created: ", user)
return true, nil //user created
Expand All @@ -108,8 +108,8 @@ func CheckIfUserExists(username string) (bool, error) {
}
defer dbConn.Close()

query := fmt.Sprint("select username from Users where username = '" + username + "'")
rows, err := dbConn.Query(query)
query := ("SELECT username FROM Users WHERE username = ?")
rows, err := dbConn.Query(query, username)
if err != nil {
return false, err
}
Expand All @@ -126,16 +126,16 @@ func InitDatabase() error {

dbConn, err := OpenDBConnection()
if err != nil {
errOpenDBConnection := fmt.Sprintf("OpenDBConnection error: %s", err)
errOpenDBConnection := ("OpenDBConnection error: %s" + err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any need to make any changes at this point. Is this change related to the vulnerability found?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has nothing to do with the vulnerability, I just understood that it would be a good practice in the code

return errors.New(errOpenDBConnection)
}

defer dbConn.Close()

queryCreate := fmt.Sprint("CREATE TABLE Users (ID int NOT NULL AUTO_INCREMENT, Username varchar(20), Password varchar(80), PRIMARY KEY (ID))")
queryCreate := ("CREATE TABLE Users (ID int NOT NULL AUTO_INCREMENT, Username varchar(20), Password varchar(80), PRIMARY KEY (ID))")
_, err = dbConn.Exec(queryCreate)
if err != nil {
errInitDB := fmt.Sprintf("InitDatabase error: %s", err)
errInitDB := ("InitDatabase error: %s" + err)
return errors.New(errInitDB)
}

Expand Down
10 changes: 10 additions & 0 deletions owasp-top10-2021-apps/a3/copy-n-paste/postRequest.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

POST /login HTTP/1.1
Host: 127.0.0.1:10001
User-Agent: curl/7.54.0
Accept: */*
Content-Type: application/json
Content-Lenght: 31

{"user":"-1' UNION SELECT 1,2,sleep(5) -- ", "pass":"password"}

5 changes: 5 additions & 0 deletions owasp-top10-2021-apps/a3/gossip-world/app/model/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# -*- coding: utf-8 -*-

import MySQLdb
import bleach


class DataBase:
Expand Down Expand Up @@ -143,6 +144,10 @@ def get_comments(self, id):
return comments, 1

def post_comment(self, author, comment, gossip_id, date):
allowed_tags = ['b', 'i', 'u', 'em', 'strong', 'a']
allowed_attrs = {'a': ['href', 'title']}

clean_comment = {bleach.clean(comment, tags=allowed_tags, attributes=allowed_attrs)}
try:
self.c.execute(
'INSERT INTO comments (author, comment, gossip_id, date) VALUES (%s, %s, %s, %s);',
Expand Down
1 change: 1 addition & 0 deletions owasp-top10-2021-apps/a3/gossip-world/app/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ mysqlclient==1.3.13
six==1.11.0
visitor==0.1.3
Werkzeug==0.14.1
bleach==5.0.1
11 changes: 7 additions & 4 deletions owasp-top10-2021-apps/a3/gossip-world/app/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import os
import uuid
import datetime
import bleach

from flask import (
Flask,
Expand All @@ -32,6 +33,8 @@
app.config['MYSQL_PASSWORD'],
app.config['MYSQL_DB'])

allowed_tags = ['b', 'i', 'u', 'em', 'strong', 'a']
allowed_attrs = {'a':['href', 'title']}

def generate_csrf_token():
'''
Expand Down Expand Up @@ -163,7 +166,7 @@ def all_gossips():
@login_required
def gossip(id):
if request.method == 'POST':
comment = request.form.get('comment')
comment = bleach.clean(request.form.get('comment'), tags=allowed_tags, attributes=allowed_attrs)
user = session.get('username')
date = datetime.datetime.now()
if comment == '':
Expand Down Expand Up @@ -198,9 +201,9 @@ def gossip(id):
@login_required
def newgossip():
if request.method == 'POST':
text = request.form.get('text')
subtitle = request.form.get('subtitle')
title = request.form.get('title')
text = bleach.clean(request.form.get('text'), tags=allowed_tags, attributes=allowed_attrs)
subtitle = bleach.clean(request.form.get('subtitle'))
title = bleach.clean(request.form.get('title'))
author = session.get('username')
date = datetime.datetime.now()
if author is None or text is None or subtitle is None or title is None:
Expand Down