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.

Sign up for GitHub

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

add field to change font color #4879

Merged
merged 9 commits into from
Aug 7, 2019
Merged

add field to change font color #4879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1728b2e
add field to change font color
octavioamu Jul 29, 2019
52118ed
remove bad migration
octavioamu Jul 29, 2019
54d2508
Merge branch 'master' into hackathon-font-color
octavioamu Jul 29, 2019
e31caa4
add migrations
octavioamu Jul 29, 2019
f28b977
Merge branch 'master' into hackathon-font-color
octavioamu Aug 7, 2019
2e13c0d
change lenght to 7
octavioamu Aug 7, 2019
ffb3826
Merge branch 'hackathon-font-color' of github.com:gitcoinco/web into …
octavioamu Aug 7, 2019
aa03b46
fix migrations
octavioamu Aug 7, 2019
7992401
Merge branch 'master' into hackathon-font-color
danlipert Aug 7, 2019
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
29 changes: 29 additions & 0 deletions app/dashboard/migrations/0046_auto_20190807_1541.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Generated by Django 2.2.3 on 2019-08-07 15:41

import django.contrib.postgres.fields
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('dashboard', '0045_auto_20190803_1827'),
]

operations = [
migrations.AddField(
model_name='hackathonevent',
name='text_color',
field=models.CharField(blank=True, help_text='hexcode for the text, default to black', max_length=7, null=True),
),
migrations.AlterField(
model_name='bounty',
name='bounty_categories',
field=django.contrib.postgres.fields.ArrayField(base_field=models.CharField(choices=[('frontend', 'frontend'), ('backend', 'backend'), ('design', 'design'), ('documentation', 'documentation'), ('other', 'other')], max_length=50), blank=True, default=list, size=None),
),
migrations.AlterField(
model_name='hackathonevent',
name='background_color',
field=models.CharField(blank=True, help_text='hexcode for the banner, default to white', max_length=7, null=True),
),
]
3 changes: 2 additions & 1 deletion app/dashboard/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3268,7 +3268,8 @@ class HackathonEvent(SuperModel):
logo_svg = models.FileField(blank=True)
start_date = models.DateTimeField()
end_date = models.DateTimeField()
background_color = models.CharField(max_length=255, null=True, blank=True, help_text='hexcode for the banner')
background_color = models.CharField(max_length=7, null=True, blank=True, help_text='hexcode for the banner, default to white')
text_color = models.CharField(max_length=7, null=True, blank=True, help_text='hexcode for the text, default to black')
identifier = models.CharField(max_length=255, default='', help_text='used for custom styling for the banner')
sponsors = models.ManyToManyField(Sponsor, through='HackathonSponsor')

Expand Down
8 changes: 3 additions & 5 deletions app/dashboard/templates/dashboard/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,15 @@
</div>
{% if hackathon %}
<link rel="stylesheet" href={% static "v2/css/hackathons/explorer.css" %}>
<div id="{{ hackathon.identifier }}" class="row pt-5 pb-5 text-center banner"
{% if hackathon.background_color %} style="background: {{ hackathon.background_color }}" {% endif %}
>
<div id="{{ hackathon.identifier }}" class="row pt-5 pb-5 text-center banner" style="background: {% firstof hackathon.background_color or 'white' %}; color: {% firstof hackathon.text_color or 'black' %};">
Copy link
Contributor

Choose a reason for hiding this comment

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

In the files above I have not seen any verification about the fact that the entered data is actually a color. How will it be verified if the entered data is not a fake string of characters that can perform a CSRF attack or break out of the default css styles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But is actually only a backend field accessible by admin. Do you think is a problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

Think about it differently.

Your approach is normal, but a bit dangerous. If all fields in the admin panel are created in this way, it means that if someone finds 1 vulnerability (administrative access) then he will be able to use some new vulnerabilities by interfering with the infrastructure. However, if the administrator's panel fields is also properly secured, someone who wrongly gains access will have less opportunities to abuse it.

Not securing something because access is only available to administrators may in the future cause a lot of problems. Increases the risk of escalating 1 bug to critical levels with the possibility of using other bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with your point but also this is only called in an inline style tag in html. So even being an open field and anyone having access I think will not open any hack opportunity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe you're right. I'm just sensitive to reporting such things. I am a bit of a pedant and that I deal with security, how do I see something like that I report :D

Seeing that I composed in my head automatically a potential route of attack from past experience

Input via color field: black; background-image('URL');

I was able to create a CSRF vulnerability, where in inline html I was able to put a marker that downloads a given url. And then perform administrative actions from the user level by including the appropriate url in it.

<div class="col g-font-muli">
<span class="hackathon-name d-block mt-5 text-white font-weight-semibold">{{ hackathon.name }}</span>
<span class="hackathon-name d-block mt-5 font-weight-semibold">{{ hackathon.name }}</span>
{% if hackathon.logo_svg %}
<img class="d-block mx-auto my-4 hackathon-logo" src="{{ hackathon.logo_svg.url }}"/>
{% elif hackathon.logo %}
<img class="d-block mx-auto my-4 hackathon-logo" src="{{ hackathon.logo.url }}"/>
{% endif %}
<span class="d-block font-title-lg text-white">
<span class="d-block font-title-lg">
{{ hackathon.start_date|date:"M j, Y" }} - {{ hackathon.end_date|date:"M j, Y" }}
</span>
</div>
Expand Down