Securely Displaying a Private Message

[doubledee]

I just built my own Private Message (PM) system, and it seems to be working great, however I am still squeamish on the security aspect...

 

(The module is like 2,000 line of code, so this will be somewhat "abstract" in nature!)

 

In order to view a PM, I run a query like this...

// Build Query.
$q2 = "SELECT member_id_to, m_to.username AS username_to, m_to.photo_name AS photo_to,
			member_id_from, m_fr.username AS username_from, m_fr.photo_name AS photo_from,
			subject, body, sent_on, read_on
		FROM private_message AS pm
		INNER JOIN pm_recipient AS r
		ON pm.id=r.message_id
		INNER JOIN member AS m_to
		ON m_to.id=r.member_id_to
		INNER JOIN member AS m_fr
		ON m_fr.id=pm.member_id_from
		WHERE r.member_id_to=?
		AND pm.id=?";

 

You can see that I need a "Member ID" and a "Message ID" to successfully find and display the PM.

 

Now, I can get the "Member ID" from the $_SESSION variable that is created when the User logs in.

 

And to get the "Message ID", when the User is in his/her Inbox and clicks on a Message summary, it takes the User to this page...

 

http://local.debbie/account/view_pm.php?msgview=incoming&msg=2

 

(I need to re-work the "msgview=incoming" part of the Query String since I don't think it even provides any value?!) 

 

Anyways, what worries me is that I am just using "msg=2" in the Query String to display the User's PM?!

 

I mean, you also need the "Member ID" and have to be logged in, but I just get this sense that maybe I am missing something here from a security standpoint?!  :-\

 

When I log in as myself and start hacking the Address bar, if I choose a Message that is not To Me or From Me, I get an Error Page that I built to handle things, so I guess that means things are working? 

 

(I think one reason I feel unsure of myself, is because of how I modeled the PM on the back end.  That is a whole other long discussion for another day!!)

 

Anyways, any thoughts on anything obviously I might be forgetting??

 

Thanks,

 

 

Debbie

 

 

[kicken]

So long as you validate that the selected message id has been sent to or from the currently logged in user then there is no problem.  The user shouldn't be able to manipulate the member id in anyway since it is in the session.

 

[requinix]

In a query string the "only" thing you need to worry about is directly revealing information. Passwords, private keys, and session IDs* do not belong in there. ID numbers are generally okay because a user doesn't know what "447" corresponds to - they'd have to visit some page so that's where you handle authorization. For example, I have a PM #69384 on this site. You can go into your PM area and stick that number in the URLs you get but you won't be able to see anything.

 

So what you have is fine. Stop worrying about it. All you have to do in the code for the page is to search for a PM that the user sent or received and that matches the ID number given. If you find it then great, and if you don't find it then tell the user they don't have that PM. Don't tell them they can't access the PM: they shouldn't know anything about it, including the fact that it exists. (Same way login forms shouldn't reveal whether a username exists - only that the username+password pair is invalid.)

 

 

(* IMO they really shouldn't, but if you take proper precautions then it's okay.)

[doubledee]

In a query string the "only" thing you need to worry about is directly revealing information. Passwords, private keys, and session IDs* do not belong in there.

 

Nor would the "Member ID", which is why I handle that in the Session.

 

 

ID numbers are generally okay because a user doesn't know what "447" corresponds to - they'd have to visit some page so that's where you handle authorization. For example, I have a PM #69384 on this site. You can go into your PM area and stick that number in the URLs you get but you won't be able to see anything.

 

Except my concern is that - and I know I'm leaving out lots of specifics here - the "Message ID" is the primary key in my "Private message" table which *is* the PM less the "To" info...

 

As stated, technically you would need the correct "Member ID" in your $_SESSION plus the correct "Message ID" in the URL to actually read a PM, but I am just being paranoid.

 

I added an extra thingy to my URL passing over whether the User was coming from the "Incoming", "Sent" or "Trash" views, but after wasting the better art of today staring at my code in a haze, I came to this conclusion...

 

If I just pass over the "Message ID" in the query string, I can then run that through an Inner Join looking in my "pm_recipient" and "private_message" tables for a "member_id_to" and/or "member_id_from" for the User's "Member ID" in their $_SESSION.

 

Then based on whether the Message is "To" the User or "From" the User, I can choose the appropriate query ending with either...

WHERE r.member_id_to=?
AND pm.id=?";

 

or

 

WHERE pm.member_id_from=?
AND pm.id=?";

 

...to display the Message.

 

So I guess I already have 1 of 2 needed pieces in the $_SESSION, and the other piece the User can pass to me in plain-sight, although I wonder if that is a good idea.  (I guess you are saying a "Message ID" isn't as damning as a Password or something?)

 

 

So what you have is fine. Stop worrying about it. All you have to do in the code for the page is to search for a PM that the user sent or received and that matches the ID number given. If you find it then great, and if you don't find it then tell the user they don't have that PM. Don't tell them they can't access the PM: they shouldn't know anything about it, including the fact that it exists. (Same way login forms shouldn't reveal whether a username exists - only that the username+password pair is invalid.)

 

(* IMO they really shouldn't, but if you take proper precautions then it's okay.)

 

 

Well, I think my errors are fairly vague like...

// Invalid Sent Message.
case 'MESSAGE_INVALID_SENT_MESSAGE_2183':
	echo	'<h1>Message Not Found</h1>';
	echo	'<p>The Private Message could not be displayed.</p>';
	echo	'<p>Please try again. (2183)</p>';
	echo '<a class="button" href="' . BASE_URL . '/account/messages.php?msgview=sent">Return to Messages</a>';
	break;

 

 

 

Debbie

 

[kicken]

but I am just being paranoid.

 

Yes, you are.

 

 

Then based on whether the Message is "To" the User or "From" the User, I can choose the appropriate query ending with either...

 

There is no need to choose a query ending.  You can use a clause like:

WHERE ? IN (m_to.id, m_fr.id) AND pm.id=?

 

That will cause the query to return a row only when the message ID matches what is in the URL and if the member id in the session matches either the to or from IDs.

 

[doubledee]

Kicken,

 

If you had a choice between this URL...

 

http://local.debbie/account/view_pm/3

 

and

 

http://local.debbie/account/view_pm/incoming/3

 

 

Which one would you pick and why?

 

I guess I like the second one because it just shows a more logical organization of a User's messages.  (Also because I just spent all weekend writing a million lines of code to support it including all error codes?!)

 

 

Debbie

 

[scootstah]

I would say the first URL, since the incoming part is irrelevant. Since the message ID is unique you don't have to specify whether it is incoming or not, you could display that information elsewhere on the page.

[kicken]

I would probably go for the first one as well.  As mentioned the incoming part is somewhat irrelevant (as far as a url goes anyway).  I would probably go for something like this instead of either but it's not that big of a deal:

 

http://local.debbie/account/messages/view/3