search

TEAMMATES / teammates

This is the project website for the TEAMMATES feedback management tool for education
https://teammatesv4.appspot.com/
GNU General Public License v2.0
1.67k stars 3.3k forks source link

Instructor: join course: validate name of institution #4854

Closed thngkaiyuan closed 4 years ago

thngkaiyuan commented 8 years ago

Currently, an instructor can amend the name of his institution upon receiving his invitation URL. This might allow situations where an instructor passes off as a staff of a reputable organisation (e.g. NUS).

Example of an attack

  1. An attacker requests for an instructor account by pretending to be a contributor-to-be
  2. After receiving his invitation URL, he amends the name of his institution in the URL to whatever he likes
  3. Since the name of the institution is not validated against the one that he provided when requesting for an account, he can then pass off as a staff of any institution
RatulGhosh commented 8 years ago

Can I use web crawler to gather information about the institution?

damithc commented 8 years ago

Not necessary. Just need to validate against the one previously provided.

RatulGhosh commented 8 years ago

Sir, we can put valid college names in DB and he/she has to select from that . Is there any other way to check the validity in real time ? Or we can ask the user to enter a college name and put it in the DB once it is verified, but I guess the latter is not real time. Sir I need some suggestion before I start solving it.

damithc commented 8 years ago

@RatulGhosh this issues requires a bit more knowledge of how things work. Why don't you start with a d.firstTimers issue or a d.Contributors issue?

RatulGhosh commented 8 years ago

sir I am looking to apply for GSoC. Can you please tell how should I contribute ? I had successfully set up the developer environment and started with #1563 and #4875.

damithc commented 8 years ago

@RatulGhosh shall we discuss the GSoC this separately as that is not related to this issue? You can open a new issue for that (yes, we use issues for discussions).

jkt001 commented 8 years ago

I'll work on this issue.

jkt001 commented 8 years ago

@damithc Based on my observation, one of the ways we can do is that we can actually store the institution name of the instructor on the Instructor entity itself, hence we don't even need to have it in the link.

If it is OK for me to change the entity and add an "Institution" field to the entity, I will go ahead.

The problem with the current one (based on my observation), is that the system might not store the institution name when the admin adds that instructor, hence it is difficult to do the comparison.

damithc commented 8 years ago

This may take a while to finish @jkt001 May be do other issues instead?

madanalogy commented 4 years ago

I would like to try and work on this if it is still an issue.

madanalogy commented 4 years ago

The implementation proposed by jkt above is a very comprehensive approach to solving this security vulnerability that I believe should be the ideal. However, it involves adding a new attribute to the instructors entity which could require extensive code rewriting elsewhere as well as potentially requiring data migration.

I will try to look at alternative methods and propose potential solutions. Do let me know however if the preferred approach is the comprehensive one, thanks!

madanalogy commented 4 years ago

Based on inspiration from @xpdavid and @ChooJeremy, I'm suggesting we generate a HMAC with the teammates secret key on the institute, then append both the institute and generate HMAC to the instructor url join link

Implementation of the HMAC will follow this gist roughly. Since the generated HMAC is a hex string, all characters will be url safe.

Do let me know if there are any issues with the solution design, otherwise I will put up a PR for this sometime next week. Thanks!