search

flpvn / wittytwitter

Automatically exported from code.google.com/p/wittytwitter
0 stars 0 forks source link

User.config contains Twitter username and password in clear text #78

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. View config file (stored in C:\Users\[username]
\AppData\Local\Witty\Witty.exe_Url_[long string]\[build number]

What is the expected output? What do you see instead?
Should not contain Twitter login information in clear text. Isn't there 
some sort of encryption support built in to config system?

Original issue reported on code.google.com by jongallo...@gmail.com on 7 Feb 2008 at 9:24

GoogleCodeExporter commented 9 years ago 
Unlike asp.net config files, encryption is not built into client app 
user.config. As
far as I know, you have to roll your own. I think the reasoning is that the 
local
desktop is more secure than files on a web host?

Will need to investigate further.

Original comment by alan...@gmail.com on 7 Feb 2008 at 5:55

GoogleCodeExporter commented 9 years ago 
This should not be a low priority defect.  At the very least, the password 
should be
encrypted using features in the System.Security namespace.

Original comment by timothyb...@gmail.com on 12 Mar 2008 at 7:53

GoogleCodeExporter commented 9 years ago 
changing it to high.

Original comment by alan...@gmail.com on 12 Mar 2008 at 8:08

GoogleCodeExporter commented 9 years ago 
anyone want to take on this bug? outside my realm of expertise.

Original comment by alan...@gmail.com on 20 Mar 2008 at 9:24

GoogleCodeExporter commented 9 years ago 
You can use DPAPI which is a painless way to store/retrieve encrypted contents 
on 
Windows 2000, XP and Vista.

http://www.obviex.com/samples/dpapi.aspx

Here is some sample code and the DLL.

Encrypt (storedPassword is your encrypted password in app.config):

if (Environment.OSVersion.Version.Major >= 6 || 
Environment.OSVersion.Version.Major 
>= 5 && Environment.OSVersion.Version.Minor > 0)
{
  try
  {
    Dpapi.DataProtector dp = new DataProtector(Dpapi.Store.UserStore);
    byte[] sourceBytes = Convert.FromBase64String(storedPassword);
    byte[] decryptedBytes = dp.Decrypt(sourceBytes);
    id = System.Text.Encoding.Unicode.GetString(decryptedBytes);
  }
}
catch {}

Decrypt:

Dpapi.DataProtector dp = new DataProtector(Dpapi.Store.UserStore);
byte[] sourceBytes = System.Text.Encoding.Unicode.GetBytes(password);
byte[] encryptedBytes = dp.Encrypt(sourceBytes);
string savedPassword = Convert.ToBase64String(encryptedBytes);

Original comment by shahi...@gmail.com on 25 Mar 2008 at 4:43

Attachments:

GoogleCodeExporter commented 9 years ago 
I think that DPAPI code is out of date; the System.Configuration system has a 
built
in DPAPI provider (DpapiProtectedConfigurationProvider)

http://msdn2.microsoft.com/en-us/library/system.configuration.dpapiprotectedconf
igurationprovider.aspx

Original comment by jongallo...@gmail.com on 25 Mar 2008 at 5:14

GoogleCodeExporter commented 9 years ago 
by golly you are right. I'm still mostly programming in .net 1.1 still.

this sounds much better, just protect parts of the app.config file.

Original comment by shahi...@gmail.com on 25 Mar 2008 at 5:22

GoogleCodeExporter commented 9 years ago 
so anyone want to own this issue :)?

Original comment by alan...@gmail.com on 25 Mar 2008 at 6:55

GoogleCodeExporter commented 9 years ago

Original comment by jongallo...@gmail.com on 25 Mar 2008 at 7:03

GoogleCodeExporter commented 9 years ago 
I'll take it

Original comment by jongallo...@gmail.com on 25 Mar 2008 at 7:04

GoogleCodeExporter commented 9 years ago 
I'm attaching a patch that encrypts the password in the config. 

I didn't just commit it just yet for two reasons:

1. I'm wondering about the upgrade scenario. If a user has an unencrypted 
password 
and runs Witty, it will try to decrypt their unencrypted password, get an 
invalid 
password, and the login will fail. I'm not sure if that's a problem, because I 
think 
the upgrade may overwrite the config, but I'm not sure.

2. In addition to encrypting the password in the configuration, I changed 
method 
signatures so we're passing around SecureString instead of System.String. It's 
much 
more secure, because unlike a System.String, it can't be sniffed in memory. 
However, 
it adds a little more complexity to the codebase so I could understand dropping 
it.

Let me know and I'll finish the changes and check it in.

Original comment by jongallo...@gmail.com on 30 Mar 2008 at 4:43

Attachments:

GoogleCodeExporter commented 9 years ago 
if login fails though it will just prompt for username password again, so that 
should work fine...

Original comment by shahi...@gmail.com on 30 Mar 2008 at 3:34

GoogleCodeExporter commented 9 years ago 
great work Jon!

I applied the patch with minor change to make the user login again when the 
password
is invalid from the settings file. Also, I don't think that the code is 
complex. only
change I would also make is to using System.Security namespace instead of fully
qualify SecureString.

Original comment by alan...@gmail.com on 30 Mar 2008 at 9:16

GoogleCodeExporter commented 9 years ago

Original comment by jongallo...@gmail.com on 3 Apr 2008 at 9:14