Skip to content

Commit 0f3723a

Browse files
bpamiribpamiri
authored
fix: SQL injection, XSS, and CSRF security hardening (#314)
fix: SQL injection, XSS, and CSRF security hardening
2 parents fc99d0e + f7465b5 commit 0f3723a

File tree

21 files changed

+206
-102
lines changed

21 files changed

+206
-102
lines changed

‎app/controllers/Controller.cfc‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ component extends="wheels.Controller" {
6565
var accesspermission = model("RolePermission").findAll(
6666
select="roleId, permissionId, name, permissionName, permissionstatus, controller, permissiondescription",
6767
include="Role, Permission",
68-
where="name = '#session.role#' AND permissions.Name = '#action#' AND permissions.controller = '#controller#'"
68+
where="name = :roleName AND permissions.Name = :actionName AND permissions.controller = :controllerName",
69+
params={roleName={value=session.role, cfsqltype="cf_sql_varchar"}, actionName={value=action, cfsqltype="cf_sql_varchar"}, controllerName={value=controller, cfsqltype="cf_sql_varchar"}}
6970
);
7071
if(accesspermission.recordCount == 0){
7172
if (structKeyExists(getHttpRequestData().headers, "HX-Request")) {
@@ -102,15 +103,17 @@ component extends="wheels.Controller" {
102103
// Shared business logic across multiple controllers
103104
public function getBlogBySlug(required string slug) {
104105
return model("Blog").findOne(
105-
where="blog_posts.slug = '#arguments.slug#' AND blog_posts.status = 'Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= '#now()#'",
106+
where="blog_posts.slug = :slug AND blog_posts.status = 'Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= :now",
107+
params={slug={value=arguments.slug, cfsqltype="cf_sql_varchar"}, now={value=now(), cfsqltype="cf_sql_timestamp"}},
106108
include="User,PostStatus",
107109
cache=10
108110
);
109111
}
110112

111113
function getTagsByBlogid(required numeric id) {
112114
return model("BlogTag").findAll(
113-
where="blogId = #arguments.id#",
115+
where="blogId = :blogId",
116+
params={blogId={value=arguments.id, cfsqltype="cf_sql_integer"}},
114117
include="Tag",
115118
cache=10
116119
);
@@ -119,7 +122,8 @@ component extends="wheels.Controller" {
119122

120123
function getCategoriesByBlogid(required numeric id) {
121124
return model("BlogCategory").findAll(
122-
where = "blogId = #arguments.id#",
125+
where = "blogId = :blogId",
126+
params = {blogId={value=arguments.id, cfsqltype="cf_sql_integer"}},
123127
include = "Blog,Category",
124128
cache = 10
125129
);
@@ -387,7 +391,7 @@ component extends="wheels.Controller" {
387391
string url = "",
388392
string isSubscriber = ""
389393
) {
390-
var emaildata = model("emailTemplate").findAll(where="title = '#arguments.templateTitle#'", cache=10);
394+
var emaildata = model("emailTemplate").findAll(where="title = :templateTitle", params={templateTitle={value=arguments.templateTitle, cfsqltype="cf_sql_varchar"}}, cache=10);
391395
if (!emaildata.recordCount) return false;
392396
var emailparams = {
393397
"name" = arguments.recipientName,
@@ -444,7 +448,8 @@ component extends="wheels.Controller" {
444448
}
445449

446450
var existingBlog = model("Blog").findFirst(
447-
where="title = '#params.title#' AND slug = '#params.slug#' AND id != #blogId#"
451+
where="title = :title AND slug = :slug AND id != :blogId",
452+
params={title={value=params.title, cfsqltype="cf_sql_varchar"}, slug={value=params.slug, cfsqltype="cf_sql_varchar"}, blogId={value=blogId, cfsqltype="cf_sql_integer"}}
448453
);
449454

450455
if (isObject(existingBlog)) {
@@ -508,7 +513,7 @@ component extends="wheels.Controller" {
508513
function deleteBlogTags(required blogId) {
509514
try {
510515
if (!isEmpty(blogId)) {
511-
model("BlogTag").deleteAll(where="blogId = #arguments.blogId#");
516+
model("BlogTag").deleteAll(where="blogId = :blogId", params={blogId={value=arguments.blogId, cfsqltype="cf_sql_integer"}});
512517
}
513518
} catch (any e) {
514519
model("Log").log(
@@ -531,7 +536,7 @@ component extends="wheels.Controller" {
531536
function deleteBlogCategories(required blogId) {
532537
try {
533538
if (!isEmpty(blogId)) {
534-
model("BlogCategory").deleteAll(where="blogId = #arguments.blogId#");
539+
model("BlogCategory").deleteAll(where="blogId = :blogId", params={blogId={value=arguments.blogId, cfsqltype="cf_sql_integer"}});
535540
}
536541
} catch (any e) {
537542
model("Log").log(

‎app/controllers/admin/NewsletterController.cfc‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ component extends="app.Controllers.Controller" {
202202
);
203203

204204
if (type == "user") {
205-
var user = model("User").findOne(where="email = '#email#'");
205+
var user = model("User").findOne(where="email = :email", params={email={value=email, cfsqltype="cf_sql_varchar"}});
206206
if (isObject(user)) {
207207
user.update(newsletter=false);
208208
model("Log").log(
@@ -227,7 +227,7 @@ component extends="app.Controllers.Controller" {
227227
};
228228
}
229229
} else {
230-
var subscriber = model("NewsletterSubscriber").findOne(where="email = '#email#'");
230+
var subscriber = model("NewsletterSubscriber").findOne(where="email = :email", params={email={value=email, cfsqltype="cf_sql_varchar"}});
231231
if (isObject(subscriber)) {
232232
subscriber.update(status="inactive");
233233
model("Log").log(
@@ -369,7 +369,11 @@ component extends="app.Controllers.Controller" {
369369

370370
if (len(trim(searchTerm))) {
371371
// Search in users table
372-
var userSubscribers = model("User").findAll(where="newsletter = 1 AND (email LIKE '%#searchTerm#%' OR firstname LIKE '%#searchTerm#%' OR lastname LIKE '%#searchTerm#%')");
372+
var searchPattern = "%" & searchTerm & "%";
373+
var userSubscribers = model("User").findAll(
374+
where="newsletter = 1 AND (email LIKE :term OR firstname LIKE :term OR lastname LIKE :term)",
375+
params={term={value=searchPattern, cfsqltype="cf_sql_varchar"}}
376+
);
373377
for (var user in userSubscribers) {
374378
subscribers.append({
375379
email: user.email,
@@ -380,7 +384,10 @@ component extends="app.Controllers.Controller" {
380384
}
381385

382386
// Search in newsletter_subscribers table
383-
var nonUserSubscribers = model("NewsletterSubscriber").findAll(where="email LIKE '%#searchTerm#%'");
387+
var nonUserSubscribers = model("NewsletterSubscriber").findAll(
388+
where="email LIKE :term",
389+
params={term={value=searchPattern, cfsqltype="cf_sql_varchar"}}
390+
);
384391
for (var subscriber in nonUserSubscribers) {
385392
subscribers.append({
386393
email: subscriber.email,

‎app/controllers/admin/RolesController.cfc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ component extends="app.Controllers.Controller" {
1818
if(id > 0) {
1919
role = model("role").findByKey(params.id);
2020
permissions = model("permission").findAll();
21-
activePermission = model("RolePermission").findAll(select="permissionId", where="roleId = #val(params.id)#");
21+
activePermission = model("RolePermission").findAll(select="permissionId", where="roleId = :roleId", params={roleId={value=val(params.id), cfsqltype="cf_sql_integer"}});
2222
existingPermissionIds = [];
2323
for (row in activePermission) {
2424
arrayAppend(existingPermissionIds, row.permissionId);
@@ -32,7 +32,7 @@ component extends="app.Controllers.Controller" {
3232
}
3333

3434
function checkRoleExistance(){
35-
var checkExistingRole = model("Role").findAll(where="name = '#params.Name#'");
35+
var checkExistingRole = model("Role").findAll(where="name = :name", params={name={value=params.Name, cfsqltype="cf_sql_varchar"}});
3636
if(checkExistingRole.recordcount != 0){
3737
renderText('<p class="fs-12 ms-2">A role already exist with this name! Role name must be unique.');
3838
return;
@@ -43,7 +43,7 @@ component extends="app.Controllers.Controller" {
4343

4444
function store(){
4545
try {
46-
var checkExistingRole = model("Role").findAll(where="name = '#params.Name#'");
46+
var checkExistingRole = model("Role").findAll(where="name = :name", params={name={value=params.Name, cfsqltype="cf_sql_varchar"}});
4747
if(checkExistingRole.recordcount != 0 && params.id == 0){
4848
redirectTo(action="index", error="A role already exist with name' #params.Name#'. Role name must be unique.");
4949
return;
@@ -85,7 +85,7 @@ component extends="app.Controllers.Controller" {
8585

8686
// Update role permissions
8787
permissionList = [];
88-
model("RolePermission").deleteAll(where="roleId = #val(RoleData.id)#");
88+
model("RolePermission").deleteAll(where="roleId = :roleId", params={roleId={value=val(RoleData.id), cfsqltype="cf_sql_integer"}});
8989
for (fieldName in RoleData) {
9090
if (left(fieldName, 11) == "permission-") {
9191
// Extract the numeric part after the dash

‎app/controllers/web/AuthController.cfc‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ component extends="app.Controllers.Controller" {
5353
}
5454
);
5555
// Check if user exists first (regardless of status)
56-
var existingUser = model("User").findOne(where="email = '#params.email#'", include="Role");
56+
var existingUser = model("User").findOne(where="email = :email", params={email={value=params.email, cfsqltype="cf_sql_varchar"}}, include="Role");
5757

5858
// If user doesn't exist, send registration invitation but return generic message
5959
if (!isObject(existingUser)) {
@@ -81,7 +81,7 @@ component extends="app.Controllers.Controller" {
8181
// Check if user is locked out
8282
if (model("LoginAttempt").isUserLocked(params.email)) {
8383
// Check if it's a manual lock by admin
84-
var user = model("User").findOne(where="email = '#params.email#'");
84+
var user = model("User").findOne(where="email = :email", params={email={value=params.email, cfsqltype="cf_sql_varchar"}});
8585
var isManuallyLocked = isObject(user) && structKeyExists(user, "locked") && user.locked;
8686

8787
model("Log").log(
@@ -303,7 +303,7 @@ component extends="app.Controllers.Controller" {
303303

304304
// Check if user needs to submit testimonial
305305
if (isObject(user.role) && user.role.name != 'Admin') {
306-
var testimonial = model("Testimonial").findOne(where="userId = #val(user.id)#");
306+
var testimonial = model("Testimonial").findOne(where="userId = :userId", params={userId={value=val(user.id), cfsqltype="cf_sql_integer"}});
307307

308308
model("Log").log(
309309
category = "wheels.auth",
@@ -369,7 +369,7 @@ component extends="app.Controllers.Controller" {
369369
if (structKeyExists(cookie, "remember_me")) {
370370
var rawToken = cookie.remember_me;
371371
var hashedToken = hash(rawToken, "SHA-256");
372-
var rememberToken = model("RememberToken").findOne(where="token = '#hashedToken#'");
372+
var rememberToken = model("RememberToken").findOne(where="token = :token", params={token={value=hashedToken, cfsqltype="cf_sql_varchar"}});
373373
if (isObject(rememberToken)) {
374374
rememberToken.delete();
375375
}
@@ -451,7 +451,7 @@ component extends="app.Controllers.Controller" {
451451
return;
452452
}
453453
// Check for duplicate email before calling saveUser
454-
var existingUser = model("User").findFirst(where="email = '#params.email#'");
454+
var existingUser = model("User").findFirst(where="email = :email", params={email={value=params.email, cfsqltype="cf_sql_varchar"}});
455455
if (isObject(existingUser)) {
456456
renderText("<p style='color:red;'>An account with this email address already exists.</p>");
457457
return;
@@ -608,7 +608,7 @@ component extends="app.Controllers.Controller" {
608608
}
609609

610610
private function validateCredentials(required string email, required string password) {
611-
var user = model("User").findOne(where="email = '#email#' AND status = 'True'", include="Role");
611+
var user = model("User").findOne(where="email = :email AND status = 'True'", params={email={value=email, cfsqltype="cf_sql_varchar"}}, include="Role");
612612
if (!isObject(user)) {
613613
return false; // User not found
614614
}
@@ -749,7 +749,7 @@ component extends="app.Controllers.Controller" {
749749
// Skip email sending in test mode
750750
return true;
751751
}
752-
var user = model("User").findOne(where="email = '#email#'");
752+
var user = model("User").findOne(where="email = :email", params={email={value=email, cfsqltype="cf_sql_varchar"}});
753753
if (!isObject(user)) return false;
754754
var verifyUrl = urlFor(action="verify", onlyPath=false) & "?token=" & token;
755755
return sendTemplateEmail("Sign Up Account Verification", user.email, user.fullname, verifyUrl);
@@ -791,7 +791,7 @@ component extends="app.Controllers.Controller" {
791791

792792
try {
793793
// Check if user already has a verification token
794-
var existingToken = model("UserToken").findOne(where="user_id = #val(user.id)# AND status = 'false'");
794+
var existingToken = model("UserToken").findOne(where="user_id = :userId AND status = 'false'", params={userId={value=val(user.id), cfsqltype="cf_sql_integer"}});
795795

796796
if (!isObject(existingToken)) {
797797
// Generate a new verification token
@@ -828,7 +828,7 @@ component extends="app.Controllers.Controller" {
828828

829829
private function verifyToken(required string token) {
830830
var message="";
831-
var tokenRecord = model("UserToken").findOne(where="token = '#token#'");
831+
var tokenRecord = model("UserToken").findOne(where="token = :token", params={token={value=token, cfsqltype="cf_sql_varchar"}});
832832

833833
if (isObject(tokenRecord)) {
834834
// Check if token has expired
@@ -854,7 +854,7 @@ component extends="app.Controllers.Controller" {
854854
}
855855

856856
private boolean function isRateLimited(required string ipAddress) {
857-
var attempts = model("LoginAttempt").findAll(where="ip_address = '#ipAddress#' AND created_at > '#dateTimeFormat(dateAdd("n", -15, now()), "yyyy-MM-dd HH:nn:ss")#'");
857+
var attempts = model("LoginAttempt").findAll(where="ip_address = :ipAddress AND created_at > :cutoff", params={ipAddress={value=ipAddress, cfsqltype="cf_sql_varchar"}, cutoff={value=dateTimeFormat(dateAdd("n", -15, now()), "yyyy-MM-dd HH:nn:ss"), cfsqltype="cf_sql_timestamp"}});
858858
return attempts.recordCount >= 3;
859859
}
860860

@@ -919,7 +919,7 @@ component extends="app.Controllers.Controller" {
919919
param name="params.email" default="";
920920

921921
try {
922-
var user = model("User").findOne(where="email = '#params.email#'");
922+
var user = model("User").findOne(where="email = :email", params={email={value=params.email, cfsqltype="cf_sql_varchar"}});
923923

924924
if (isObject(user)) {
925925
// Generate reset token
@@ -971,7 +971,8 @@ component extends="app.Controllers.Controller" {
971971

972972
try {
973973
var reset = model("PasswordReset").findOne(
974-
where="token = '#params.token#' AND expiresAt > '#dateTimeFormat(now(), "yyyy-MM-dd HH:nn:ss")#' AND used = 0"
974+
where="token = :token AND expiresAt > :now AND used = 0",
975+
params={token={value=params.token, cfsqltype="cf_sql_varchar"}, now={value=dateTimeFormat(now(), "yyyy-MM-dd HH:nn:ss"), cfsqltype="cf_sql_timestamp"}}
975976
);
976977

977978
if (!isObject(reset)) {
@@ -1004,7 +1005,8 @@ component extends="app.Controllers.Controller" {
10041005
try {
10051006
// Validate token
10061007
var reset = model("PasswordReset").findOne(
1007-
where="token = '#params.token#' AND expiresAt > '#dateTimeFormat(now(), "yyyy-MM-dd HH:nn:ss")#' AND used = 0"
1008+
where="token = :token AND expiresAt > :now AND used = 0",
1009+
params={token={value=params.token, cfsqltype="cf_sql_varchar"}, now={value=dateTimeFormat(now(), "yyyy-MM-dd HH:nn:ss"), cfsqltype="cf_sql_timestamp"}}
10081010
);
10091011

10101012
if (!isObject(reset)) {

‎app/controllers/web/BlogController.cfc‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ component extends="app.Controllers.Controller" {
348348
return Val(authorParam);
349349
} else {
350350
// Lookup user by username
351-
var user = model("user").findOne(where = "username = '#arguments.authorParam#'");
351+
var user = model("user").findOne(where = "username = :username", params={username={value=arguments.authorParam, cfsqltype="cf_sql_varchar"}});
352352
if (IsObject(user)) {
353353
return user.id;
354354
} else {
@@ -374,7 +374,8 @@ component extends="app.Controllers.Controller" {
374374
if (Len(Trim(searchTerm))) {
375375
var searchPattern = "%#searchTerm#%";
376376
var query = model("blog").findAll(
377-
where = "blog_posts.status ='Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= '#Now()#' AND (blog_posts.slug LIKE '#searchPattern#' OR blog_posts.title LIKE '#searchPattern#' OR blog_posts.content LIKE '#searchPattern#' OR fullname LIKE '#searchPattern#' OR email LIKE '#searchPattern#')",
377+
where = "blog_posts.status ='Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= '#Now()#' AND (blog_posts.slug LIKE :pattern OR blog_posts.title LIKE :pattern OR blog_posts.content LIKE :pattern OR fullname LIKE :pattern OR email LIKE :pattern)",
378+
params = {pattern={value=searchPattern, cfsqltype="cf_sql_varchar"}},
378379
include = "User, PostStatus, PostType",
379380
order = "publishedAt DESC",
380381
page = page,
@@ -384,7 +385,8 @@ component extends="app.Controllers.Controller" {
384385
if (isInfiniteScroll) {
385386
totalCount = model("blog").count(
386387
include = "User, PostStatus, PostType",
387-
where = "blog_posts.status ='Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= '#Now()#' AND (blog_posts.slug LIKE '#searchPattern#' OR blog_posts.title LIKE '#searchPattern#' OR blog_posts.content LIKE '#searchPattern#' OR fullname LIKE '#searchPattern#' OR email LIKE '#searchPattern#')"
388+
where = "blog_posts.status ='Approved' AND blog_posts.publishedAt IS NOT NULL AND blog_posts.publishedAt <= '#Now()#' AND (blog_posts.slug LIKE :pattern OR blog_posts.title LIKE :pattern OR blog_posts.content LIKE :pattern OR fullname LIKE :pattern OR email LIKE :pattern)",
389+
params = {pattern={value=searchPattern, cfsqltype="cf_sql_varchar"}}
388390
);
389391
hasMore = (page * perPage) < totalCount;
390392
isSearched = true;
@@ -619,7 +621,7 @@ component extends="app.Controllers.Controller" {
619621

620622
// Allow title change and check uniqueness
621623
if (StructKeyExists(params, "title")) {
622-
var existingBlog = model("Blog").findFirst(where = "title = '#params.title#' AND id != #blogId#");
624+
var existingBlog = model("Blog").findFirst(where = "title = :title AND id != :blogId", params={title={value=params.title, cfsqltype="cf_sql_varchar"}, blogId={value=blogId, cfsqltype="cf_sql_integer"}});
623625
if (IsObject(existingBlog)) {
624626
result.success = false;
625627
result.message = "A blog post with this title already exists.";
@@ -706,13 +708,15 @@ component extends="app.Controllers.Controller" {
706708
);
707709

708710
if (StructKeyExists(form, "title")) {
709-
var whereClause = "title = '#form.title#'";
711+
var queryParams = {title={value=form.title, cfsqltype="cf_sql_varchar"}};
712+
var whereClause = "title = :title";
710713

711714
if (StructKeyExists(form, "id") && IsNumeric(form.id) && form.id > 0) {
712-
whereClause &= " AND id != #form.id#";
715+
whereClause &= " AND id != :formId";
716+
queryParams.formId = {value=form.id, cfsqltype="cf_sql_integer"};
713717
}
714718

715-
var blogModel = model("Blog").findAll(where = whereClause);
719+
var blogModel = model("Blog").findAll(where = whereClause, params = queryParams);
716720

717721
if (blogModel.recordCount != 0) {
718722
renderText('<span class="text-danger">A blog already exists with this title!</span><input type="hidden" id="titleExists" value="1">');

0 commit comments

Comments
 (0)