[ISSUE-67901957] address comments, prevent SQL injection
diff --git a/accessEntity/data.go b/accessEntity/data.go index 233e7c5..50fcd93 100644 --- a/accessEntity/data.go +++ b/accessEntity/data.go
@@ -22,7 +22,6 @@ const ( sql_select_api_product = `SELECT * FROM kms_api_product AS ap ` - sql_select_org = `SELECT * FROM kms_organization AS o WHERE o.tenant_id=$1 LIMIT 1;` sql_select_tenant_org = ` (SELECT o.tenant_id FROM kms_organization AS o WHERE o.name=?)` ) @@ -36,7 +35,7 @@ case TypeConsumerKey: query = selectApiProductsById( selectAppCredentialMapperByConsumerKey( - "'"+id+"'", + "?", "apiprdt_id", ), "name", @@ -44,7 +43,7 @@ case TypeApp: query = selectApiProductsById( selectAppCredentialMapperByAppId( - "'"+id+"'", + "?", "apiprdt_id", ), "name", @@ -53,7 +52,7 @@ return nil, fmt.Errorf("unsupported idType") } - rows, err := d.GetDb().Query(query) + rows, err := d.GetDb().Query(query, id) if err != nil { return nil, err } @@ -74,11 +73,11 @@ func (d *DbManager) GetComNameByComId(comId string) (string, error) { query := selectCompanyByComId( - "'"+comId+"'", + "?", "name", ) name := sql.NullString{} - err := d.GetDb().QueryRow(query).Scan(&name) + err := d.GetDb().QueryRow(query, comId).Scan(&name) if err != nil || !name.Valid { return "", err } @@ -87,11 +86,11 @@ func (d *DbManager) GetDevEmailByDevId(devId string) (string, error) { query := selectDeveloperById( - "'"+devId+"'", + "?", "email", ) email := sql.NullString{} - err := d.GetDb().QueryRow(query).Scan(&email) + err := d.GetDb().QueryRow(query, devId).Scan(&email) if err != nil || !email.Valid { return "", err } @@ -104,21 +103,21 @@ case TypeDeveloper: query = selectCompanyByComId( selectCompanyDeveloperByDevId( - "'"+id+"'", + "?", "company_id", ), "name", ) case TypeCompany: query = selectCompanyByComId( - "'"+id+"'", + "?", "name", ) default: return nil, fmt.Errorf("unsupported idType") } - rows, err := d.GetDb().Query(query) + rows, err := d.GetDb().Query(query, id) if err != nil { return nil, err } @@ -142,18 +141,18 @@ switch t { case TypeDeveloper: query = selectAppByDevId( - "'"+id+"'", + "?", "name", ) case TypeCompany: query = selectAppByComId( - "'"+id+"'", + "?", "name", ) default: return nil, fmt.Errorf("app type not supported") } - rows, err := d.GetDb().Query(query) + rows, err := d.GetDb().Query(query, id) if err != nil { return nil, err } @@ -177,17 +176,17 @@ switch t { case AppTypeDeveloper: query = selectDeveloperById( - "'"+id+"'", + "?", "status", ) case AppTypeCompany: query = selectCompanyByComId( - "'"+id+"'", + "?", "status", ) } status := sql.NullString{} - err := d.GetDb().QueryRow(query).Scan(&status) + err := d.GetDb().QueryRow(query, id).Scan(&status) if err != nil || !status.Valid { return "", err } @@ -315,13 +314,13 @@ cols := []string{"*"} query := selectApiProductsById( selectAppCredentialMapperByAppId( - "'"+appId+"'", + "?", "apiprdt_id", ), cols..., ) + " AND ap.tenant_id IN " + sql_select_tenant_org //log.Debugf("getApiProductsByAppId: %v", query) - err = d.GetDb().QueryStructs(&apiProducts, query, org) + err = d.GetDb().QueryStructs(&apiProducts, query, appId, org) return } @@ -329,51 +328,57 @@ cols := []string{"*"} query := selectApiProductsById( selectAppCredentialMapperByConsumerKey( - "'"+consumerKey+"'", + "?", "apiprdt_id", ), cols..., ) + " AND ap.tenant_id IN " + sql_select_tenant_org //log.Debugf("getApiProductsByConsumerKey: %v", query) - err = d.GetDb().QueryStructs(&apiProducts, query, org) + err = d.GetDb().QueryStructs(&apiProducts, query, consumerKey, org) return } func (d *DbManager) getApiProductsByAppName(appName, devEmail, devId, comName, org string) (apiProducts []common.ApiProduct, err error) { cols := []string{"*"} var appQuery string + args := []interface{}{appName} switch { case devEmail != "": appQuery = selectAppByNameAndDeveloperId( - "'"+appName+"'", + "?", selectDeveloperByEmail( - "'"+devEmail+"'", + "?", "id", ), "id", ) + args = append(args, devEmail) case devId != "": appQuery = selectAppByNameAndDeveloperId( - "'"+appName+"'", - "'"+devId+"'", + "?", + "?", "id", ) + args = append(args, devId) case comName != "": appQuery = selectAppByNameAndCompanyId( - "'"+appName+"'", + "?", selectCompanyByName( - "'"+comName+"'", + "?", "id", ), "id", ) + args = append(args, comName) default: appQuery = selectAppByName( - "'"+appName+"'", + "?", "id", ) } + args = append(args, org) + query := selectApiProductsById( selectAppCredentialMapperByAppId( appQuery, @@ -382,58 +387,65 @@ cols..., ) + " AND ap.tenant_id IN " + sql_select_tenant_org //log.Debugf("getApiProductsByAppName: %v", query) - err = d.GetDb().QueryStructs(&apiProducts, query, org) + err = d.GetDb().QueryStructs(&apiProducts, query, args...) return } func (d *DbManager) getAppByAppId(id, org string) (apps []common.App, err error) { cols := []string{"*"} query := selectAppById( - "'"+id+"'", + "?", cols..., ) + " AND a.tenant_id IN " + sql_select_tenant_org - //log.Debugf("getAppByAppId: %v", query) - err = d.GetDb().QueryStructs(&apps, query, org) + //log.Debugf("getAppByAppId: %v \n %v", query, id) + err = d.GetDb().QueryStructs(&apps, query, id, org) return } func (d *DbManager) getAppByAppName(appName, devEmail, devId, comName, org string) (apps []common.App, err error) { cols := []string{"*"} var query string + args := []interface{}{appName} switch { case devEmail != "": query = selectAppByNameAndDeveloperId( - "'"+appName+"'", + "?", selectDeveloperByEmail( - "'"+devEmail+"'", + "?", "id", ), cols..., ) + args = append(args, devEmail) case devId != "": query = selectAppByNameAndDeveloperId( - "'"+appName+"'", - "'"+devId+"'", + "?", + "?", cols..., ) + args = append(args, devId) case comName != "": query = selectAppByNameAndCompanyId( - "'"+appName+"'", + "?", selectCompanyByName( - "'"+comName+"'", + "?", "id", ), cols..., ) + args = append(args, comName) default: query = selectAppByName( - "'"+appName+"'", + "?", cols..., ) } + + args = append(args, org) + query += " AND a.tenant_id IN " + sql_select_tenant_org //log.Debugf("getAppByAppName: %v", query) - err = d.GetDb().QueryStructs(&apps, query, org) + err = d.GetDb().QueryStructs(&apps, query, args...) return } @@ -441,24 +453,24 @@ cols := []string{"*"} query := selectAppById( selectAppCredentialMapperByConsumerKey( - "'"+consumerKey+"'", + "?", "app_id", ), cols..., ) + " AND a.tenant_id IN " + sql_select_tenant_org //log.Debugf("getAppByConsumerKey: %v", query) - err = d.GetDb().QueryStructs(&apps, query, org) + err = d.GetDb().QueryStructs(&apps, query, consumerKey, org) return } func (d *DbManager) getAppCredentialByConsumerKey(consumerKey, org string) (appCredentials []common.AppCredential, err error) { cols := []string{"*"} query := selectAppCredentialByConsumerKey( - "'"+consumerKey+"'", + "?", cols..., ) + " AND ac.tenant_id IN " + sql_select_tenant_org //log.Debugf("getAppCredentialByConsumerKey: %v", query) - err = d.GetDb().QueryStructs(&appCredentials, query, org) + err = d.GetDb().QueryStructs(&appCredentials, query, consumerKey, org) return } @@ -466,13 +478,13 @@ cols := []string{"*"} query := selectAppCredentialByConsumerKey( selectAppCredentialMapperByAppId( - "'"+appId+"'", + "?", "appcred_id", ), cols..., ) + " AND ac.tenant_id IN " + sql_select_tenant_org //log.Debugf("getAppCredentialByAppId: %v", query) - err = d.GetDb().QueryStructs(&appCredentials, query, org) + err = d.GetDb().QueryStructs(&appCredentials, query, appId, org) return } @@ -480,24 +492,24 @@ cols := []string{"*"} query := selectCompanyByComId( selectAppById( - "'"+appId+"'", + "?", "company_id", ), cols..., ) + " AND com.tenant_id IN " + sql_select_tenant_org //log.Debugf("getCompanyByAppId: %v", query) - err = d.GetDb().QueryStructs(&companies, query, org) + err = d.GetDb().QueryStructs(&companies, query, appId, org) return } func (d *DbManager) getCompanyByName(name, org string) (companies []common.Company, err error) { cols := []string{"*"} query := selectCompanyByName( - "'"+name+"'", + "?", cols..., ) + " AND com.tenant_id IN " + sql_select_tenant_org //log.Debugf("getCompanyByName: %v", query) - err = d.GetDb().QueryStructs(&companies, query, org) + err = d.GetDb().QueryStructs(&companies, query, name, org) return } @@ -506,7 +518,7 @@ query := selectCompanyByComId( selectAppById( selectAppCredentialMapperByConsumerKey( - "'"+consumerKey+"'", + "?", "app_id", ), "company_id", @@ -514,7 +526,7 @@ cols..., ) + " AND com.tenant_id IN " + sql_select_tenant_org //log.Debugf("getCompanyByConsumerKey: %v", query) - err = d.GetDb().QueryStructs(&companies, query, org) + err = d.GetDb().QueryStructs(&companies, query, consumerKey, org) return } @@ -522,13 +534,13 @@ cols := []string{"*"} query := selectCompanyDeveloperByComId( selectCompanyByName( - "'"+comName+"'", + "?", "id", ), cols..., ) + " AND cd.tenant_id IN " + sql_select_tenant_org //log.Debugf("getCompanyDeveloperByComName: %v", query) - err = d.GetDb().QueryStructs(&companyDevelopers, query, org) + err = d.GetDb().QueryStructs(&companyDevelopers, query, comName, org) return } @@ -536,13 +548,13 @@ cols := []string{"*"} query := selectDeveloperById( selectAppById( - "'"+appId+"'", + "?", "developer_id", ), cols..., ) + " AND dev.tenant_id IN " + sql_select_tenant_org //log.Debugf("getDeveloperByAppId: %v", query) - err = d.GetDb().QueryStructs(&developers, query, org) + err = d.GetDb().QueryStructs(&developers, query, appId, org) return } @@ -551,7 +563,7 @@ query := selectDeveloperById( selectAppById( selectAppCredentialMapperByConsumerKey( - "'"+consumerKey+"'", + "?", "app_id", ), "developer_id", @@ -559,29 +571,29 @@ cols..., ) + " AND dev.tenant_id IN " + sql_select_tenant_org //log.Debugf("getDeveloperByConsumerKey: %v", query) - err = d.GetDb().QueryStructs(&developers, query, org) + err = d.GetDb().QueryStructs(&developers, query, consumerKey, org) return } func (d *DbManager) getDeveloperByEmail(email, org string) (developers []common.Developer, err error) { cols := []string{"*"} query := selectDeveloperByEmail( - "'"+email+"'", + "?", cols..., ) + " AND dev.tenant_id IN " + sql_select_tenant_org //log.Debugf("getDeveloperByEmail: %v", query) - err = d.GetDb().QueryStructs(&developers, query, org) + err = d.GetDb().QueryStructs(&developers, query, email, org) return } func (d *DbManager) getDeveloperById(id, org string) (developers []common.Developer, err error) { cols := []string{"*"} query := selectDeveloperById( - "'"+id+"'", + "?", cols..., ) + " AND dev.tenant_id IN " + sql_select_tenant_org //log.Debugf("getDeveloperById: %v", query) - err = d.GetDb().QueryStructs(&developers, query, org) + err = d.GetDb().QueryStructs(&developers, query, id, org) return }
diff --git a/accessEntity/data_test.go b/accessEntity/data_test.go index 59d1823..e0e873f 100644 --- a/accessEntity/data_test.go +++ b/accessEntity/data_test.go
@@ -24,6 +24,10 @@ const ( fileDataTest = "data_test.sql" + // SQL Injection + // If select foo from bar where id in (' + condition + ') is used, + // the hacked sql would be like "select XXX from XXX where id in ('1') or ('1'=='1');" + sqlInjectionStmt = "1') or ('1'=='1" ) var _ = Describe("DataTest", func() { @@ -73,6 +77,15 @@ {IdentifierAppName, "non-existent", IdentifierDeveloperEmail, "non-existent", "apid-haoming"}, {IdentifierAppName, "non-existent", IdentifierCompanyName, "non-existent", "apid-haoming"}, {IdentifierAppName, "non-existent", IdentifierApiResource, "non-existent", "apid-haoming"}, + // SQL Injection + {IdentifierApiProductName, "apstest", "", "", sqlInjectionStmt}, + {IdentifierApiProductName, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppId, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppName, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierConsumerKey, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppName, "apstest", IdentifierDeveloperId, sqlInjectionStmt, "apid-haoming"}, + {IdentifierAppName, "testappahhis", IdentifierDeveloperEmail, sqlInjectionStmt, "apid-haoming"}, + {IdentifierAppName, "apstest", IdentifierCompanyName, sqlInjectionStmt, "apid-haoming"}, } var expectedDevApiProd = common.ApiProduct{ @@ -137,6 +150,14 @@ nil, nil, nil, + nil, + nil, + nil, + nil, + nil, + nil, + nil, + nil, } for i, data := range testData { @@ -168,6 +189,14 @@ {IdentifierAppName, "non-existent", IdentifierDeveloperEmail, "non-existent", "apid-haoming"}, {IdentifierAppName, "non-existent", IdentifierCompanyName, "non-existent", "apid-haoming"}, {IdentifierConsumerKey, "non-existent", "", "", "apid-haoming"}, + // SQL Injection + {IdentifierAppId, "408ad853-3fa0-402f-90ee-103de98d71a5", "", "", sqlInjectionStmt}, + {IdentifierAppId, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppName, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppName, "apstest", IdentifierDeveloperId, sqlInjectionStmt, "apid-haoming"}, + {IdentifierAppName, "apstest", IdentifierDeveloperEmail, sqlInjectionStmt, "apid-haoming"}, + {IdentifierAppName, "testappahhis", IdentifierCompanyName, sqlInjectionStmt, "apid-haoming"}, + {IdentifierConsumerKey, sqlInjectionStmt, "", "", "apid-haoming"}, } var expectedDevApp = common.App{ @@ -222,6 +251,13 @@ nil, nil, nil, + nil, + nil, + nil, + nil, + nil, + nil, + nil, } for i, data := range testData { @@ -247,6 +283,11 @@ {IdentifierAppId, "non-existent", "", "", "apid-haoming"}, {IdentifierCompanyName, "non-existent", "", "", "apid-haoming"}, {IdentifierConsumerKey, "non-existent", "", "", "apid-haoming"}, + // SQL Injection + {IdentifierAppId, "35608afe-2715-4064-bb4d-3cbb4e82c474", "", "", sqlInjectionStmt}, + {IdentifierAppId, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierCompanyName, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierConsumerKey, sqlInjectionStmt, "", "", "apid-haoming"}, } var expectedCom = common.Company{ @@ -269,6 +310,10 @@ nil, nil, nil, + nil, + nil, + nil, + nil, } for i, data := range testData { @@ -297,6 +342,12 @@ {IdentifierConsumerKey, "non-existent", "", "", "apid-haoming"}, {IdentifierDeveloperEmail, "non-existent", "", "", "apid-haoming"}, {IdentifierDeveloperId, "non-existent", "", "", "apid-haoming"}, + // SQL Injection + {IdentifierAppId, "408ad853-3fa0-402f-90ee-103de98d71a5", "", "", sqlInjectionStmt}, + {IdentifierAppId, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierConsumerKey, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierDeveloperEmail, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierDeveloperId, sqlInjectionStmt, "", "", "apid-haoming"}, } var expectedDev = common.Developer{ @@ -326,6 +377,11 @@ nil, nil, nil, + nil, + nil, + nil, + nil, + nil, } for i, data := range testData { @@ -349,6 +405,10 @@ {IdentifierConsumerKey, "abcd", "", "", "non-existent"}, {IdentifierConsumerKey, "non-existent", "", "", "apid-haoming"}, {IdentifierAppId, "non-existent", "", "", "apid-haoming"}, + // SQL Injection + {IdentifierConsumerKey, "abcd", "", "", sqlInjectionStmt}, + {IdentifierConsumerKey, sqlInjectionStmt, "", "", "apid-haoming"}, + {IdentifierAppId, sqlInjectionStmt, "", "", "apid-haoming"}, } var expectedCred = common.AppCredential{ @@ -374,6 +434,9 @@ nil, nil, nil, + nil, + nil, + nil, } for i, data := range testData { @@ -392,10 +455,12 @@ testData := [][]string{ // positive tests {IdentifierCompanyName, "testcompanyhflxv", "", "", "apid-haoming"}, - // negative tests {IdentifierCompanyName, "testcompanyhflxv", "", "", "non-existent"}, {IdentifierCompanyName, "non-existent", "", "", "apid-haoming"}, + // SQL Injection + {IdentifierCompanyName, "testcompanyhflxv", "", "", sqlInjectionStmt}, + {IdentifierCompanyName, sqlInjectionStmt, "", "", "apid-haoming"}, } var expectedComDev = common.CompanyDeveloper{ @@ -413,6 +478,8 @@ {expectedComDev}, nil, nil, + nil, + nil, } for i, data := range testData {