Prevent parameters in POST overriding other values

[mdjnelson]

Users can modify parameters when going from a consumer site to a publisher site using LTI. There is some protection against this by signing a request using a secret available to the publisher and consumer sites. However, this is signing can be worked around by modifying the request to the publisher.

[mdjnelson]

I spoke to @claudevervoort about this and he +1ed the patch.

[spvickers]

I do not see the issue here. I believe the OAuth code used in this library does not support signed requests which have parameters on the query string and in the post data with the same name. If such a request is received the signature check will fail. The only possible modification which could be made in transit would be for a user to change the location of a parameter from the query string to the post data (or vice versa) but the name and value of the parameter would have to remain the same. Am I missing something?

[mdjnelson]

Hi Stephen,

Thanks for the response.

I hope the reproduction steps below help.

Prerequisite steps

1. Install software that will intercepts HTTP requests before they go out and allows full editing. Browser extensions were no good for me. Check out software called Burpsuite.
2. Create a course on one site, we'll call this the publisher site. Enable LTI authentication and LTI enrolment. Go to the course and publish this as an LTI tool. Just leave role for teacher as its default, which should be non-editing teacher
3. Create a course on another site, we'll call this the consumer site. Create an LTI activity and enter the details as necessary to set this up so that it links to the course on the publisher site.
4. Enrol a student in the consumer site course.
5. You can log in as the user and make sure that you can legitimately use the LTI activity on the consumer site to become enrolled in the course on the publisher site. (Sometimes there might need to be some re-checking/adjusting of settings on either site to get this working). They should be enrolled as a Student in the publisher site course.

To carry out the exploit:

1. Turn intercepting on in Burpsuite. Enable the proxy in your browser so that requests can be intercepted.
2. As a student, click the lti activity on the consumer site.
3. 'Forward' other requests in Burpsuite until you get to the request to the tool.php script on the publisher site.
4. Take the post parameters and stick them at the end of the url (they need to be in the same order).
5. Change one of the letters in the Content-Type to uppercase, e.g. application/x-www-form-urlencodeD
6. Make sure to leave the GET parameters you've just added to the URL as they are, but you can modify the POST params to your liking. in this case, change the role from 'Learner' to 'Instructor'
7. Click 'Forward' in Burpsuite to let that modified request go out.

Outcome

The student will now also be enrolled as a non-editing teacher on the publisher site.

There are other parameters in the request that you could modify, including 'user_id'. Which could allow you to log in as another lti user on the publisher site.

[spvickers]

OK, thanks for the example, though I am not sure the order of the parameters in the query string should matter as this should not affect the signature.

Would this issue not be fixed by just using stristr instead of strstr when checking the value of the Content-Type header?

Is there also a potential issue with the case of the name of the Content-Type header or does PHP normalise the header names. I will experiment if no one else has.

[claudevervoort]

i'm no php expert but my initial reaction was I think the same as yours Stephen, just make the comparison case insensitive. However Mark showed me the other patch and it shrugs on the content type and handle POST params if there are any. That seemed even better to me, why care about content type at all if there are POST params?