From 48b399e05601d64c771600bd0f48cfe20d7a3fde Mon Sep 17 00:00:00 2001 From: Rene Schallner Date: Wed, 24 May 2023 01:28:04 +0200 Subject: [PATCH] fix mem leaks in UserPassSessionAuth --- .../userpass_session_auth.zig | 93 +++++++++++-------- src/http_auth.zig | 29 ++++-- 2 files changed, 75 insertions(+), 47 deletions(-) diff --git a/examples/userpass_session_auth/userpass_session_auth.zig b/examples/userpass_session_auth/userpass_session_auth.zig index f3b9c2f..4acdc0e 100644 --- a/examples/userpass_session_auth/userpass_session_auth.zig +++ b/examples/userpass_session_auth/userpass_session_auth.zig @@ -85,6 +85,14 @@ fn on_request(r: zap.SimpleRequest) void { return on_logout(r); } + // /logout can be shown since we're still authenticated for this + // very request + if (std.mem.startsWith(u8, p, "/stop")) { + std.log.info(" + for /stop --> logout page", .{}); + zap.stop(); + return on_logout(r); + } + // any other paths will still show the normal page std.log.info(" + --> normal page", .{}); return on_normal_page(r); @@ -100,48 +108,59 @@ pub fn main() !void { var gpa = std.heap.GeneralPurposeAllocator(.{ .thread_safe = true, }){}; - var allocator = gpa.allocator(); - var listener = zap.SimpleHttpListener.init(.{ - .port = 3000, - .on_request = on_request, - .log = true, - .max_clients = 100000, - }); - try listener.listen(); + // we start a block here so the defers will run before we call the gpa + // to detect leaks + { + var allocator = gpa.allocator(); + var listener = zap.SimpleHttpListener.init(.{ + .port = 3000, + .on_request = on_request, + .log = true, + .max_clients = 100000, + }); + try listener.listen(); - zap.enableDebugLog(); + zap.enableDebugLog(); - // add a single user to our allowed users - var userpass = Lookup.init(allocator); - try userpass.put("zap", "awesome"); + // add a single user to our allowed users + var userpass = Lookup.init(allocator); + defer userpass.deinit(); + try userpass.put("zap", "awesome"); - // init our auth - authenticator = try Authenticator.init( - allocator, - &userpass, - .{ - .usernameParam = "username", - .passwordParam = "password", - .loginPage = loginpath, - .cookieName = "zap-session", - }, - ); + // init our auth + authenticator = try Authenticator.init( + allocator, + &userpass, + .{ + .usernameParam = "username", + .passwordParam = "password", + .loginPage = loginpath, + .cookieName = "zap-session", + }, + ); + defer authenticator.deinit(); - // just some debug output: listing the session tokens the authenticator may - // have generated already (if auth_lock_token_table == false). - const lookup = authenticator.sessionTokens; - std.debug.print("\nauth token list len: {d}\n", .{lookup.count()}); - var it = lookup.iterator(); - while (it.next()) |item| { - std.debug.print(" {s}\n", .{item.key_ptr.*}); + // just some debug output: listing the session tokens the authenticator may + // have generated already (if auth_lock_token_table == false). + const lookup = authenticator.sessionTokens; + std.debug.print("\nauth token list len: {d}\n", .{lookup.count()}); + var it = lookup.iterator(); + while (it.next()) |item| { + std.debug.print(" {s}\n", .{item.key_ptr.*}); + } + + std.debug.print("Visit me on http://127.0.0.1:3000\n", .{}); + + // start worker threads + zap.start(.{ + .threads = 2, + .workers = 1, + }); } - std.debug.print("Visit me on http://127.0.0.1:3000\n", .{}); - - // start worker threads - zap.start(.{ - .threads = 2, - .workers = 1, - }); + // all defers should have run by now + std.debug.print("\n\nSTOPPED!\n\n", .{}); + const leaked = gpa.detectLeaks(); + std.debug.print("Leaks detected: {}\n", .{leaked}); } diff --git a/src/http_auth.zig b/src/http_auth.zig index c96592f..eca59aa 100644 --- a/src/http_auth.zig +++ b/src/http_auth.zig @@ -395,12 +395,27 @@ pub fn UserPassSessionAuth(comptime Lookup: type, comptime lockedPwLookups: bool while (it.next()) |kv| { // we iterate over all usernames and passwords, create tokens, // and memorize the tokens - _ = try ret.createAndStoreSessionToken(kv.key_ptr.*, kv.value_ptr.*); + const token = try ret.createAndStoreSessionToken(kv.key_ptr.*, kv.value_ptr.*); + allocator.free(token); } } return ret; } + pub fn deinit(self: *Self) void { + self.allocator.free(self.settings.usernameParam); + self.allocator.free(self.settings.passwordParam); + self.allocator.free(self.settings.loginPage); + self.allocator.free(self.settings.cookieName); + + // clean up the session tokens: the key strings are duped + var key_it = self.sessionTokens.keyIterator(); + while (key_it.next()) |key_ptr| { + self.allocator.free(key_ptr.*); + } + self.sessionTokens.deinit(); + } + /// Check for session token cookie, remove the token from the valid tokens /// Note: only valid if lockedTokenLookups == true pub fn logout(self: *Self, r: *const zap.SimpleRequest) void { @@ -447,13 +462,6 @@ pub fn UserPassSessionAuth(comptime Lookup: type, comptime lockedPwLookups: bool } } - pub fn deinit(self: *const Self) void { - self.allocator.free(self.settings.usernameParam); - self.allocator.free(self.settings.passwordParam); - self.allocator.free(self.settings.loginPage); - self.allocator.free(self.settings.cookieName); - } - fn _internal_authenticateRequest(self: *Self, r: *const zap.SimpleRequest) AuthResult { // if we're requesting the login page, let the request through if (r.path) |p| { @@ -521,6 +529,7 @@ pub fn UserPassSessionAuth(comptime Lookup: type, comptime lockedPwLookups: bool if (std.mem.eql(u8, pw.str, correct_pw)) { // create session token if (self.createAndStoreSessionToken(username.str, pw.str)) |token| { + defer self.allocator.free(token); // now set the cookie header if (r.setCookie(.{ .name = self.settings.cookieName, @@ -594,11 +603,11 @@ pub fn UserPassSessionAuth(comptime Lookup: type, comptime lockedPwLookups: bool defer self.tokenLookupLock.unlock(); if (!self.sessionTokens.contains(token)) { - try self.sessionTokens.put(token, {}); + try self.sessionTokens.put(try self.allocator.dupe(u8, token), {}); } } else { if (!self.sessionTokens.contains(token)) { - try self.sessionTokens.put(token, {}); + try self.sessionTokens.put(try self.allocator.dupe(u8, token), {}); } } return token;