0

I have a WebService with the following Method:

    [ScriptMethod(ResponseFormat = ResponseFormat.Json)]
    [WebMethod]
    public string Login(string passwort, string email, string firma)
    {
        return LoginHelper.Login(passwort, email, firma);
    }

My LoginHelper code:

    using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Data;
using System.Data.SqlClient;

namespace WebService1
{
    public class LoginHelper
    {
        public static string Login(string passwort, string email, string firma)
        {
            string userName = "";

            SqlConnection con = new SqlConnection(@"Data Source=Yeah-PC\SQLEXPRESS;Initial Catalog=works;Integrated Security=true;");

        SqlCommand cmd = new SqlCommand(@"SELECT firma FROM TestData 
                                  WHERE email = @email", con);
        cmd.Parameters.AddWithValue("@email", email);

        con.Open();

        SqlDataReader dr = cmd.ExecuteReader();
                while (dr.Read())
      {
   //userName += dr["email"].ToString();
   //userName += dr["passwort"].ToString();
   userName += dr["firma"].ToString();
     }
    dr.Close();
    con.Close();
    return userName;
        }



    }
}

Thanks for that help Guys

I have edited my Questions. Is that solution secure now? I mean against SQL-Injection. Is there something more what i can do better?

Bashud
  • 266
  • 2
  • 8
  • 20
  • 5
    The first thing you're doing wrong is concatenating SQL strings. You should use query parameters instead. – David May 30 '12 at 15:04
  • 1
    You should use parameterized SQL for this; your code as currently written is vulnerable to SQL injection attack. I would make that change first; there's a chance the problem will go away when you do that. – Robert Harvey May 30 '12 at 15:05
  • 2
    You're also failing to put your SqlConnection, SqlCommand, and SqlDataReader in `using` blocks, and you're using ASMX web services when you should be using WCF unless you have no choice. – John Saunders May 30 '12 at 15:07
  • @David can you help me a little? Iam new at this and i dont know how to use query parameters – Bashud May 30 '12 at 15:09
  • 1
    @Bashud: http://blog.divergencehosting.com/2009/04/09/using-parameters-parameterized-queries-database-interactions-cshar-vbnet/ – Robert Harvey May 30 '12 at 15:11

5 Answers5

6

you are calling LoginHelper.Login(passwort, email, firma);

but in your method

public static string Login(string email, string passwort, string firma)

email is fist parameter.

actually in email parameter you have password, that's why it not return any result

change your login method in LoginHelper as below

public static string Login(string passwort, string email, string firma)
{
    string userName = "";

    using (SqlConnection con = new SqlConnection(@"Data Source=Yeah-PC\SQLEXPRESS;Initial Catalog=works;Integrated Security=true;"))
    using(SqlCommand cmd = new SqlCommand(@"SELECT firma FROM TestData WHERE email = @email", con))
    {
        cmd.Parameters.AddWithValue("@email", email);
        con.Open();
        using (SqlDataReader rdr = cmd.ExecuteReader())
        {
            while (rdr.Read())
            {
                if (rdr["firma"]  != DBNull.Value)
                {
                    userName += rdr["firma"].ToString();
                }

            }
        }
    }

    return userName;
}
Damith
  • 62,401
  • 13
  • 102
  • 153
3

If your email address contains an @ character, that may be your problem. The @ is a parameter marker for SQLCommand. It is going to think the latter part of your e-mail address is a sql parameter. You will need to pass the e-mail address using a parameter. That also protects you from SQL injection. Akatakritos's answer has an example of how to pass the email as a prameter.

user957902
  • 3,010
  • 14
  • 18
1

Also, for security and performance reasons, you should be using SqlParameters. Read up on SQL Injection attacks.

string userName = "";

SqlConnection con = new SqlConnection(@"Data Source=Yeah-PC\SQLEXPRESS;Initial Catalog=works;Integrated Security=true;");

SqlCommand cmd = new SqlCommand(@"SELECT firma FROM TestData 
                                  WHERE email = @email" con);
cmd.Parameters.AddWithValue("@email", email);

con.Open();

SqlDataReader dr = cmd.ExecuteReader();
while (dr.Read())
{
   //userName += dr["email"].ToString();
   //userName += dr["passwort"].ToString();
   userName += dr["firma"].ToString();
}
dr.Close();
con.Close();
return userName;
akatakritos
  • 9,836
  • 1
  • 23
  • 29
0

Per the other answer & comments.

You have a security issue, if you're not going to use an ORM (Entity Framework/NHibernate/etc...) please use parameterized queries

Solving your problem:

  • Is data in your database?
  • Are you pointing at the right database?
  • Is your SQL correct?
  • Is your SQL getting run?
  • Run SQL Profiler and see what SQL is getting run, then test that in your SQL Management Studio
Nathan Koop
  • 24,803
  • 25
  • 90
  • 125
0

Instead of passing parameter in code try to use Sqlparameter to add parameter. It is best practice to use SQL parameter to add parametrs. You can also check the value of email by debugging....wether you are passing correct information.

        SqlConnection conn = new SqlConnection(connectionString);
        conn.Open();
        SqlCommand cmd = new SqlCommand(@"SELECT firma FROM TestData 
                              WHERE email = @email" conn);
        cmd.Parameters.AddWithValue("@email", email);                     
        cmd.Prepare();
        cmd.ExecuteNonQuery();
        SqlDataReader dr = cmd.ExecuteReader();
        while (dr.Read())
            {                  
               userName += dr["firma"].ToString();

            }
        dr.Close();
        conn.Close();
Chetan S
  • 935
  • 1
  • 11
  • 21
  • the informations iam passing are correct, i already checked that. Damith posted the Solution – Bashud May 30 '12 at 15:23
  • i could solved my Problem with the solution from Damith, but iam grateful for your help too :) – Bashud May 30 '12 at 15:39