臨下班接到通報,有段 Entity Framework 4 的老程式被偵測出有 SQL Injection 漏洞:

var res = ctx.ExecuteStoreQuery<Player>(
    "SELECT * FROM Player WHERE UserName = {0}", userName);

我第一時間的反應是「靠! 是哪個王八蛋? 都宣導多少次了還在寫 SQL Injection,被我抓到是誰就準備被阿魯巴到死吧...」,調出版控修改歷程很快揪出犯人 - 就是我本人。

天哪,我當時到底在想什麼呀? 不敢相信自己會犯這麼低級的錯,但鐵證如山,事到如今我別無選擇了,從活動櫃拿出珍藏多年的窖藏 12 年單一純麥巴拉刈,扭開瓶蓋之際忽然想到,等等,這又不是 string.Format("SELECT ... {0}", varName),它是 ExcecuteStoreQuery<T>(),在還沒有 Dapper 可用的 EF4 時代,這是在 EF 直接執行 SQL 語法的捷徑。而依我一直以來的認知,{0} {1} 會被轉成資料參數物件,微軟也不至於設計誘人誤陷 SQL Injection 的 API,所以我當年才敢大膽使用。

偏偏當年我沒有留下 KB 可以印證這點,更要命的是我還查到 ExecuteStoreQuery("...{0}...", varName) 可能導致 SQL Injection 的說法。頓時一陣涼意從背脊一路涼上後腦杓,這下大條了,莫非我多年來認知有錯,無意間已埋下無數 SQL Injection 地雷?

此事非同小可,我先是查了 ObjectContext.ExecuteStoreQuery<T>() 原始碼,其背後是呼叫 CreateStoreCommand(string commandText, params object[] parameters) 建立 DbCommand。若 object[] parameters 不含任何 DbParamter 物件(!parameters.Any(p => p is DbParameter))時,會自動將 parameters 以 command.CreateParameter() 轉成 DbParameter 陣列,再用 string.Format() 將指令中的 {0} {1}... 轉成 @p0、@p1...,故 parameters 背後仍是以參數方式執行,並無 SQL Injection 疑慮。

private DbCommand CreateStoreCommand(string commandText, params object[] parameters)
        {
            DbCommand command = this._connection.StoreConnection.CreateCommand();
            command.CommandText = commandText;

            // get relevant state from the object context
            if (this.CommandTimeout.HasValue)
            {
                command.CommandTimeout = this.CommandTimeout.Value;
            }
            EntityTransaction entityTransaction = this._connection.CurrentTransaction;
            if (null != entityTransaction)
            {
                command.Transaction = entityTransaction.StoreTransaction;
            }

            if (null != parameters && parameters.Length > 0)
            {
                DbParameter[] dbParameters = new DbParameter[parameters.Length];

                // three cases: all explicit DbParameters, no explicit DbParameters
                // or a mix of the two (throw in the last case)
                if (parameters.All(p => p is DbParameter))
                {
                    for (int i = 0; i < parameters.Length; i++)
                    {
                        dbParameters[i] = (DbParameter)parameters[i];
                    }
                }
                else if (!parameters.Any(p => p is DbParameter))
                {
                    string[] parameterNames = new string[parameters.Length];
                    string[] parameterSql = new string[parameters.Length];
                    for (int i = 0; i < parameters.Length; i++)
                    {
                        parameterNames[i] = string.Format(CultureInfo.InvariantCulture, "p{0}", i);
                        dbParameters[i] = command.CreateParameter();
                        dbParameters[i].ParameterName = parameterNames[i];
                        dbParameters[i].Value = parameters[i] ?? DBNull.Value;

                        // By default, we attempt to swap in a SQL Server friendly representation of the parameter.
                        // For other providers, users may write:
                        //
                        //      ExecuteStoreQuery("select * from foo f where f.X = ?", 1);
                        //
                        // rather than:
                        //
                        //      ExecuteStoreQuery("select * from foo f where f.X = {0}", 1);
                        parameterSql[i] = "@" + parameterNames[i];
                    }
                    command.CommandText = string.Format(CultureInfo.InvariantCulture, command.CommandText, parameterSql);
                }
                else
                {
                    throw EntityUtil.InvalidOperation(System.Data.Entity.Strings.ObjectContext_ExecuteCommandWithMixOfDbParameterAndValues);
                }

                command.Parameters.AddRange(dbParameters);
            }

            return command;
        }

分析完程式碼還是不太放心,想親手實測求證。但 ExecuteStoreQuery 是 EF4 時代的方法,VS2017 專案的 ADO.NET Data Entity Model 從 EF5.0 起跳,除非回頭安裝 VS2010,很難新增 EF4 專案。所幸在 CodeProject 我找到一篇 Entity Framework Tutorial for Beginners 有 VS2010 建立的可下載專案範例,稍作修改後終於能重現 ExecuteStoreQuery<T>,並透過 SQL Profiler 驗證參數陣列有被傳成 @p0。

做完實驗印證自己的認知無誤,一堆 ExecuteStoreQuery 寫法安全無虞,這才鬆了一口氣。

EF4 已是快十年的活化石,新版做法已改無 ExecuteStoreQuery 可用。而現在再有類似需求,我會義無反顧抄起 Dapper 上場,故瞎忙一整晚的心得價值並不高,但卻能讓我安心睡覺,哈!

Research of how EF4 ExecuteStoreQuery convert {0} {1} and paramters to clarify if it will cause SQL inejction.


Comments

# by Ike

所以是源碼檢查的誤判嗎?

# by Jeffrey

to lke, 檢查軟體應是看到{0} {1}就開槍,但經檢視原始碼跟實測都無法證明有SQL Injection的可能,本案例可視為誤判。

Post a comment