RiiConnect24 / RiiTag

RiiTag is a customizable gamertag for the Wii.
https://tag.rc24.xyz/
GNU Affero General Public License v3.0
14 stars 7 forks source link

Validate Inputs #33

Open TheShadowEevee opened 3 years ago

TheShadowEevee commented 3 years ago

Issue As it stands, input's don't seem to be validated at all for RiiTags when editing. This is a problem as people can put whatever they want in some fields. This can range from unlimited text that will overflow on the tag image and cause bigger tag config sizes, to people being able to move between directories on the server to pick a background.

Severity: Low Technically this can't be used maliciously other than causing huge file sizes with large copy-pastes and causing server-side errors, seeing as in order to directory hop you need to know both what the directory tree looks like AND the file name you're looking for, and even then you can only display the image as a background.

Why this needs to be fixed There are two types of people to note here:

  1. People with access to the RiiTag Edit page: These people can type whatever they want into the open boxes such as the Nickname, Wii No., and Game ID's

  2. People who know how to create post requests and want to mess with their tag: If you know how to create post requests (cURL makes it easy if you know the command) it's a simple exercise to do whatever you want with you RiiTag.

{Note: Example RiiTag has been reverted and removed from this issue, everything below still applys} For Example:

While I specifically used the background as an example here, the same applies to the flag, and all other form values.

Possible Fix The best way to fix this varies based on the field. Wii number and nickname just needs to be limited in characters for example, while background selection and other similar fields needs to be limited so that people can't directory jump.

How I've tested this I tested this using both a local copy of the RiiTag server and the public version at https://tag.rc24.xyz

I can also include the cURL command I used for most of this, but for now I'm leaving it out to keep people from messing with their tags due to the server-side error's I've seen this can cause. (Many errors in my localhost testing, plus RiiTag as a whole froze for a few seconds during testing on the public server, oops!)

larsenv commented 3 years ago

Thank you for making this well-written issue. I never thought about people modifying the POST requests when editing their tag. I see how having the background be a path could be a security issue that can easily be fixed.

We could implement a 32-character limit for the name and Wii Number. There is a way to verify if a Wii Number is valid, but I wouldn't want to use it for this in the case people want to put notes or other things in that field.

Also, we only use SQL for storing user IDs with the token to use RiiTag. The user data is stored as JSON files on disk (which worries me!)

Thanks.

bennyman123abc commented 3 years ago

Sorry for the late reply. This issue will be fixed by the end of the night, completely ignoring Larsen's comment about Wii Number needing all characters. That has been an issue for a while, and I never got around to fix it as I never thought it would be a bigger issue.

Thank you for this report!

larsenv commented 3 years ago

@bennyman123abc @TheShadowEevee Should we close this?

But wait - Ben, it didn't seem you committed a fix for this issue...