SQL injection in login/Membership authentication (SCADA/Program/CoreTest/MemberPrincipal.cs) allows auth bypass #42

Summary

SCADA/Program/CoreTest/MemberPrincipal.cs builds every authentication-related SQL statement (Authenticate, UpdatePassword, the MemberPrincipal constructor's role lookup, and CreateUser) by string-concatenating the attacker-supplied username directly into raw SQL text, executed unparameterized via SqlCommand. The only guard is a client-side UI length check (user.Length > 10) in Login.xaml.cs -- it doesn't strip or escape SQL metacharacters, so a 9-character payload is enough to bypass login.

Root cause

MemberPrincipal.cs:94 (Authenticate):

string sql = "SELECT Password FROM dbo.Membership WHERE Username='" + _name + "'";
using (var reader = DataHelper.Instance.ExecuteReader(sql))
{
    while (reader.Read())
    {
        string pass = reader.GetString(0);
        if (pass == EncodePassword(password)) { _isAuthenticated = true; return 0; }
        else { _count++; return -1; }
    }
}

Same pattern at MemberPrincipal.cs:38-39 (role lookup) and :143-146/:165,177 (CreateUser/UpdatePassword, no length limit at all -- these also allow stacked statements since string.Format joins two statements with ;).

The only input check is Login.xaml.cs:21-26, a client-side length-only gate:

string user = txtUser.Text;
if (string.IsNullOrEmpty(user) || user.Length > 10) { ... return; }
identity = new MemberIdentity(user);
int rs = identity.Authenticate(txtPassword.Password);

MssqlFactory.ExecuteReader (SCADA/Program/DataHelper/MssqlFactory.cs:321-328) passes the string straight into SqlCommand's CommandText with no parameters, and SqlClient batches semicolon-separated statements by default.

The bypass

Username ' or ''=' (9 chars, passes the <=10 UI gate) turns the query into a tautology:

SELECT Password FROM dbo.Membership WHERE Username='' or ''=''

This matches every row in Membership. The code only inspects the first row returned and compares its hash to EncodePassword(<submitted password>). This project ships a documented default account (op/1, per the login error strings). Submitting password 1 against the tautology payload authenticates as whichever account the query returns first, without knowing any real username -- turning "log in as a specific user" into "log in as some row in the table."

Verification

Isolated-algorithm check: extracted the exact SQL-building logic and the exact UI length gate verbatim into a standalone C# program (mcs/mono), confirming the emitted SQL text is exactly the tautology above for the 9-char payload.

Also independently re-verified end-to-end against a real SQL engine (SQLite via Mono.Data.Sqlite, substituting for SqlServer since that's what was available) running the real Authenticate()/EncodePassword() bodies verbatim, seeded with the default op/1 row plus a second account:

• exploit: username=' or ''=', password='1' -> Authenticate returns 0 (authenticated) -- the bypass.
• control: username='no_such_user', password='1' -> returns -2 (no rows, not authenticated).
• sanity: username='op', password='1' (the real default account) -> returns 0 (authenticated).

Impact

Unauthenticated SQL injection directly reachable from the login screen. Practical impact is bounded by the 10-char UI gate (limits classic UNION-based exfiltration from this specific entry point) and the code only consuming the first returned row, but it's a real, unauthenticated auth-bypass primitive plus a broader unparameterized-SQL pattern (CreateUser/UpdatePassword) that would be trivially exploitable via stacked queries by any caller not subject to the UI's length restriction.

Suggested fix

• Use parameterized queries (SqlParameter) for every statement in MemberPrincipal.cs, the same way ExecuteProcedureReader already does elsewhere via DataHelper.CreateParam.
• Don't rely on client-side length checks as an injection defense.
• Filter server-side by an indexed, parameterized Username=@user rather than trusting "first row returned."
• Avoid Integrated Security=True with an over-privileged local account for this table.

Duplicate check

security-advisories API returns []. Reviewed all 41 issues -- none reference SQL injection, authentication, or Membership/login.