tunapanda / funzo-app

Cordova app running ember and h5p
2 stars 3 forks source link

Possible security issue: `isTeacher` can be modified via browser console? #74

Closed usernamenumber closed 6 years ago

usernamenumber commented 8 years ago
  1. Install and enable the Ember Chrome extension, if needed
  2. Load funzo
  3. Open the Ember inspector
  4. Click Data, then your user
  5. In the data pane on the right, change isTeacher (or anything else)

The change doesn't seem to survive a reload of the page, but it does seem to stick around until then (e.g. if I inspect another record and then go back to the user, the changed value remains changed).

@Jakeii, is this the inspector's UI being misleading, or an actual concern?

Jakeii commented 8 years ago

this can be persisted by calling model.save() in the console, there is no way to prevent it. But can you give an example where it would be a concern?

usernamenumber commented 8 years ago

Well, I assume isTeacher is intended to eventually manage some kind of access control, right? Otherwise why distinguish between teachers and non-teachers? In that case, isn't it essentially privilege escalation if someone can simply change that value client-side?

On Monday, June 6, 2016, Jake Lee Kennedy notifications@github.com wrote:

this can be persisted by calling model.save() in the console, there is no way to prevent it. But can you give an example where it would be a concern?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tunapanda/funzo-app/issues/74#issuecomment-223967423, or mute the thread https://github.com/notifications/unsubscribe/AALfvQXYdoiTzWN0HnFjRn7a3YAiRv_2ks5qJCf5gaJpZM4IubDV .

Jakeii commented 8 years ago

Well only the teacher account can get the settings ui for managing courses, xapi statements, and user accounts

On Mon, 6 Jun 2016, 19:48 Brad Smith, notifications@github.com wrote:

Well, I assume isTeacher is intended to eventually manage some kind of access control, right? Otherwise why distinguish between teachers and non-teachers? In that case, isn't it essentially privilege escalation if someone can simply change that value client-side?

On Monday, June 6, 2016, Jake Lee Kennedy notifications@github.com wrote:

this can be persisted by calling model.save() in the console, there is no way to prevent it. But can you give an example where it would be a concern?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/tunapanda/funzo-app/issues/74#issuecomment-223967423 , or mute the thread < https://github.com/notifications/unsubscribe/AALfvQXYdoiTzWN0HnFjRn7a3YAiRv_2ks5qJCf5gaJpZM4IubDV

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tunapanda/funzo-app/issues/74#issuecomment-224017118, or mute the thread https://github.com/notifications/unsubscribe/ABpqTl5rwQNUOo1lFRKz8BPg5KW3uTQ6ks5qJE9FgaJpZM4IubDV .

usernamenumber commented 8 years ago

Given that the xapi statements represent a student's participation records, quiz scores (eventually), etc, isn't that a problem? An enterprising student could just change isTeacher and then alter their results however they like, right?

--Brad

Jakeii commented 8 years ago

You can't use the remote inspector on production apps, so it won't be much of an issue there.

But any access to the console will let a user do anything they want, including finding the LRS api key and sending whatever statements they want.

On Mon, 6 Jun 2016, 23:59 Brad Smith, notifications@github.com wrote:

Given that the xapi statements represent a student's participation records, quiz scores (eventually), etc, isn't that a problem? An enterprising student could just change isTeacher and then alter their results however they like, right?

--Brad

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tunapanda/funzo-app/issues/74#issuecomment-224086658, or mute the thread https://github.com/notifications/unsubscribe/ABpqTsZ8PZWp0owmxIX7iEWmz1rTL15Eks5qJIo8gaJpZM4IubDV .