From 5a7552af858f5be0b77573e4a18e87d7221ab7f4 Mon Sep 17 00:00:00 2001
From: Devin Hussey <husseydevin@gmail.com>
Date: Thu, 28 Feb 2019 14:21:37 -0500
Subject: [PATCH] Use a larger table and a better hash function to prevent
 flooding

dash's current addition based hash is very weak (not even good enough for
"non-cryptographic"). It isn't even that fast, especially when things start
flooding.

    unsigned hashval = (unsigned char)str[0] << 4;

    while (*str) {
        hashval += (unsigned char)*str++;
    }

Being addition-based, hashvar and __lookupalias are incredibly weak to the
commutative property (a + b == b + a). Any permutation after the first byte
is going to result in the same hash value, which means collision.

Permutations can be generated very easily, and unlike other hashes, brute force
isn't even required:

Creating a 120000 line script that sets a bunch of variables with names that
are permutations of each other takes a second or so with std::next_permutation.

    abcdefghijkl=
    abcdefghijlk=
    abcdefghikjl=
    acbdefghiklj=
    abcdefghiljk=
    ...

On my 2.0 GHz 2nd Gen Core i7, this locks up dash for 1-2 minutes, compared to
Bash, ZSH, mksh, and ksh which take about 1-2 seconds each,

This could be used maliciously to create a denial-of-service attack via hash
flooding that basically affects most Linux/BSD distros.

What I changed:

I replaced it with a better hash, GoodOAAT from SMHasher, which is a one byte at a
time hash function that actually passes SMHasher. While it isn't the strongest hash
out there, it is probably the best for our purpose, and either way, it isn't a piece
of garbage that can be exploited with a 4th grade education.

The reason I chose a one-at-a-time hash is because variable names are typically
going to be small and we are not provided with the length.

I added an FNVmod function to the hash file that shows how any hash function can
be dropped in, however, it is disabled by default.

I seeded the hash functions with either /dev/urandom (preferred) or the CPU time
from clock(). This makes it far less likely that a static script can cause a
flooding attack.

Any argument about performance is moot, if I can generate a script that takes
two minutes to parse compared to other shells parsing it in a few seconds,
that is a problem.

Additionally, it is 2019. Most systems have at least 4 GB RAM, and we don't need
to be so stingy with the hash table sizes. A max of 39 variables can be created,
then the table is forced to chain. That is absurd. For comparison, dash's
(rather simple) configure script sets about 400 variables. That means that on
average, each bucket will have 10 entries by the end of the script.

I decided to use sizes closer to what Bash uses, 61 for aliases and 1021 for
variables which is much more reasonable and still pretty conservative.
Unfortunately, being a one at a time hash, the natural bias requires
that the hash table be prime.

I also stored the actual hash value for the variables and aliases. It doesn't
use up any extra memory on 64-bit machines because the flags field forces 4
bytes of padding, and it lets us do a quick equality check before bringing out
the byte-by-byte comparison function.

With that done, dash went from hanging for two minutes on that malicious script
to parsing it in 0.7 seconds, which is even faster than Bash.

I also changed the var tables to use NULL instead of 0 because I needed to update
them anyways.

Signed-off-by: Devin Hussey <husseydevin@gmail.com>
---
 src/Makefile.am |   4 +-
 src/alias.c     |  35 +++++-----
 src/alias.h     |   3 +
 src/hash.c      | 170 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/hash.h      |  41 ++++++++++++
 src/var.c       | 103 ++++++++++++++++-------------
 src/var.h       |   2 +
 7 files changed, 292 insertions(+), 66 deletions(-)
 create mode 100644 src/hash.c
 create mode 100644 src/hash.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 1732465..bfe2953 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,14 +18,14 @@ bin_PROGRAMS = dash
 
 dash_CFILES = \
 	alias.c arith_yacc.c arith_yylex.c cd.c error.c eval.c exec.c expand.c \
-	histedit.c input.c jobs.c mail.c main.c memalloc.c miscbltin.c \
+	hash.c histedit.c input.c jobs.c mail.c main.c memalloc.c miscbltin.c \
 	mystring.c options.c parser.c redir.c show.c trap.c output.c \
 	bltin/printf.c system.c bltin/test.c bltin/times.c var.c
 dash_SOURCES = \
 	$(dash_CFILES) \
 	alias.h arith_yacc.h bltin/bltin.h cd.h error.h eval.h exec.h \
 	expand.h \
-	init.h input.h jobs.h machdep.h mail.h main.h memalloc.h miscbltin.h \
+	hash.h init.h input.h jobs.h machdep.h mail.h main.h memalloc.h miscbltin.h \
 	myhistedit.h mystring.h options.h output.h parser.h redir.h shell.h \
 	show.h system.h trap.h var.h
 dash_LDADD = builtins.o init.o nodes.o signames.o syntax.o
diff --git a/src/alias.c b/src/alias.c
index daeacbb..547c671 100644
--- a/src/alias.c
+++ b/src/alias.c
@@ -41,22 +41,25 @@
 #include "mystring.h"
 #include "alias.h"
 #include "options.h"	/* XXX for argptr (should remove?) */
+#include "hash.h"
 
-#define ATABSIZE 39
+/* Should be less than 0xFFFF and prime for the hash. */
+#define ATABSIZE 61
 
 struct alias *atab[ATABSIZE];
 
 STATIC void setalias(const char *, const char *);
 STATIC struct alias *freealias(struct alias *);
-STATIC struct alias **__lookupalias(const char *);
+STATIC struct alias **__lookupalias(const char *, uint32_t *out /* nullable */);
 
 STATIC
 void
 setalias(const char *name, const char *val)
 {
 	struct alias *ap, **app;
+	uint32_t hash;
 
-	app = __lookupalias(name);
+	app = __lookupalias(name, &hash);
 	ap = *app;
 	INTOFF;
 	if (ap) {
@@ -70,6 +73,7 @@ setalias(const char *name, const char *val)
 		ap = ckmalloc(sizeof (struct alias));
 		ap->name = savestr(name);
 		ap->val = savestr(val);
+		ap->hash = hash;
 		ap->flag = 0;
 		ap->next = 0;
 		*app = ap;
@@ -82,7 +86,7 @@ unalias(const char *name)
 {
 	struct alias **app;
 
-	app = __lookupalias(name);
+	app = __lookupalias(name, NULL);
 
 	if (*app) {
 		INTOFF;
@@ -116,7 +120,7 @@ rmaliases(void)
 struct alias *
 lookupalias(const char *name, int check)
 {
-	struct alias *ap = *__lookupalias(name);
+	struct alias *ap = *__lookupalias(name, NULL);
 
 	if (check && ap && (ap->flag & ALIASINUSE))
 		return (NULL);
@@ -144,7 +148,7 @@ aliascmd(int argc, char **argv)
 	}
 	while ((n = *++argv) != NULL) {
 		if ((v = strchr(n+1, '=')) == NULL) { /* n+1: funny ksh stuff */
-			if ((ap = *__lookupalias(n)) == NULL) {
+			if ((ap = *__lookupalias(n, NULL)) == NULL) {
 				outfmt(out2, "%s: %s not found\n", "alias", n);
 				ret = 1;
 			} else
@@ -200,25 +204,20 @@ printalias(const struct alias *ap) {
 	out1fmt("%s=%s\n", ap->name, single_quote(ap->val));
 }
 
+
 STATIC struct alias **
-__lookupalias(const char *name) {
-	unsigned int hashval;
+__lookupalias(const char *name, uint32_t *hash_out) {
+	uint32_t hashval;
 	struct alias **app;
-	const char *p;
-	unsigned int ch;
 
-	p = name;
+	hashval = hash_alias(name);
+	if (hash_out != NULL)
+		*hash_out = hashval;
 
-	ch = (unsigned char)*p;
-	hashval = ch << 4;
-	while (ch) {
-		hashval += ch;
-		ch = (unsigned char)*++p;
-	}
 	app = &atab[hashval % ATABSIZE];
 
 	for (; *app; app = &(*app)->next) {
-		if (equal(name, (*app)->name)) {
+		if ((*app)->hash == hashval && equal(name, (*app)->name)) {
 			break;
 		}
 	}
diff --git a/src/alias.h b/src/alias.h
index fb841d6..97e8f15 100644
--- a/src/alias.h
+++ b/src/alias.h
@@ -37,11 +37,14 @@
 #define ALIASINUSE	1
 #define ALIASDEAD	2
 
+#include <stdint.h>
+
 struct alias {
 	struct alias *next;
 	char *name;
 	char *val;
 	int flag;
+	uint32_t hash;
 };
 
 struct alias *lookupalias(const char *, int);
diff --git a/src/hash.c b/src/hash.c
new file mode 100644
index 0000000..7015954
--- /dev/null
+++ b/src/hash.c
@@ -0,0 +1,170 @@
+/*-
+ * Copyright (c) 2019
+ *	Herbert Xu <herbert@gondor.apana.org.au>.  All rights reserved.
+ *
+ * This code is derived from software contributed to Berkeley by
+ * Kenneth Almquist.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <time.h>
+
+#include "shell.h"
+#include "hash.h"
+
+static uint32_t hash_seed;      /* seed for hash tables */
+
+/*
+ * Reads some data from /dev/urandom to serve as a random
+ * seed. If it can't get data from /dev/urandom for some reason,
+ * we use clock() instead.
+ *
+ * This makes the hash table a lot less predictable.
+ *
+ * The constructor attribute causes this to run at startup.
+ */
+__attribute__((constructor))
+static void generate_seed(void)
+{
+	FILE *f = fopen("/dev/urandom", "rb");
+
+	if (f != NULL) {
+		uint64_t urandom;
+		fread(&urandom, 1, sizeof(urandom), f);
+		fclose(f);
+		hash_seed = urandom ^ (urandom >> 32);
+	} else {
+		/* Better than nothing. */
+		uint64_t ret = (uint64_t)clock();
+		hash_seed = ret ^ (ret >> 32);
+	}
+#ifdef HASHTABLE_DEBUG
+	TRACE(("SEED: %08X\n", hash_seed));
+#endif
+}
+
+/* Use any hash functions you wish. The only requirement is that
+ * hash_alias hashes until it finds a null byte and hash_var hashes
+ * until a null byte or an '='. */
+#ifdef USE_FNV
+
+/*
+ * Modified FNV1a with better avalanching.
+ * http://papa.bretmulvey.com/post/124027987928/hash-functions
+ */
+uint32_t hash_alias(const char *name)
+{
+	const uint32_t prime = 16777619U;
+	uint32_t hash = 2166136261U ^ hash_seed;
+	while (*name)
+		hash = (hash ^ (uint8_t)*name++) * prime;
+	hash += hash << 13;
+	hash ^= hash >> 7;
+	hash += hash << 3;
+	hash ^= hash >> 17;
+	hash += hash << 5;
+	return hash;
+}
+
+uint32_t hash_var(const char *name)
+{
+	const uint32_t prime = 16777619U;
+	uint32_t hash = 2166136261U ^ hash_seed;
+	while (*name && *name != '=')
+		hash = (hash ^ (uint8_t)*name++) * prime;
+	hash += hash << 13;
+	hash ^= hash >> 7;
+	hash += hash << 3;
+	hash ^= hash >> 17;
+	hash += hash << 5;
+	return hash;
+}
+
+#else
+#define grol(x,n) (((x)<<(n))|((x)>>(32-(n))))
+#define gror(x,n) (((x)>>(n))|((x)<<(32-(n))))
+
+/*
+ * GoodOAAT hash from SMHasher
+ *
+ * One of the smallest non-multiplicative One-At-a-Time function
+ * that passes the whole SMHasher.
+ * Author: Sokolov Yura aka funny-falcon <funny.falcon@gmail.com>
+ */
+uint32_t hash_alias(const char *name)
+{
+	const unsigned char  *str = (const unsigned char *)name;
+	uint32_t h1 = hash_seed ^ 0x3b00;
+	uint32_t h2 = grol(hash_seed, 15);
+	while (*str) {
+		h1 += *str++;
+		h1 += h1 << 3; /* h1 *= 9 */
+		h2 += h1;
+		/* the rest could be as in MicroOAAT: h1 = grol(h1, 7)
+		 * but clang doesn't generate ROTL instruction then. */
+		h2 = grol(h2, 7);
+		h2 += h2 << 2; /* h2 *= 5 */
+	}
+	h1 ^= h2;
+	/* now h1 passes all collision checks,
+	 * so it is suitable for hash-tables with prime numbers. */
+	h1 += grol(h2, 14);
+	h2 ^= h1; h2 += gror(h1, 6);
+	h1 ^= h2; h1 += grol(h2, 5);
+	h2 ^= h1; h2 += gror(h1, 8);
+	return h2;
+}
+
+uint32_t hash_var(const char *name)
+{
+	const unsigned char  *str = (const unsigned char *)name;
+	uint32_t h1 = hash_seed ^ 0x3b00;
+	uint32_t h2 = grol(hash_seed, 15);
+	while (*str && *str != '=') {
+		h1 += *str++;
+		h1 += h1 << 3; /* h1 *= 9 */
+		h2 += h1;
+		/* the rest could be as in MicroOAAT: h1 = grol(h1, 7)
+		 * but clang doesn't generate ROTL instruction then. */
+		h2 = grol(h2, 7);
+		h2 += h2 << 2; /* h2 *= 5 */
+	}
+	h1 ^= h2;
+	/* now h1 passes all collision checks,
+	 * so it is suitable for hash-tables with prime numbers. */
+	h1 += grol(h2, 14);
+	h2 ^= h1; h2 += gror(h1, 6);
+	h1 ^= h2; h1 += grol(h2, 5);
+	h2 ^= h1; h2 += gror(h1, 8);
+	return h2;
+}
+
+#undef grol
+#undef gror
+
+#endif
diff --git a/src/hash.h b/src/hash.h
new file mode 100644
index 0000000..e144352
--- /dev/null
+++ b/src/hash.h
@@ -0,0 +1,41 @@
+/*-
+ * Copyright (c) 2019
+ *	Herbert Xu <herbert@gondor.apana.org.au>.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ */
+
+#ifndef HASH_H
+#define HASH_H
+
+#include <stdint.h>
+
+/* A hash function that hashes a C string until '\0'. */
+uint32_t hash_alias(const char *name);
+/* A hash function that hashes a C string until '=' or '\0'. */
+uint32_t hash_var(const char *name);
+
+#endif /* HASH_H */
diff --git a/src/var.c b/src/var.c
index 0d7e1db..8add372 100644
--- a/src/var.c
+++ b/src/var.c
@@ -61,9 +61,10 @@
 #include "myhistedit.h"
 #endif
 #include "system.h"
+#include "hash.h"
 
-
-#define VTABSIZE 39
+/* Should be smaller than 0xFFFF and prime to work for the hash. */
+#define VTABSIZE 1021
 
 
 struct localvar_list {
@@ -84,30 +85,29 @@ char linenovar[sizeof("LINENO=")+sizeof(int)*CHAR_BIT/3+1] = "LINENO=";
 /* Some macros in var.h depend on the order, add new variables to the end. */
 struct var varinit[] = {
 #if ATTY
-	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"ATTY\0",	0 },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"ATTY\0",	NULL },
 #endif
-	{ 0,	VSTRFIXED|VTEXTFIXED,		defifsvar,	0 },
-	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAIL\0",	changemail },
-	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAILPATH\0",	changemail },
-	{ 0,	VSTRFIXED|VTEXTFIXED,		defpathvar,	changepath },
-	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS1=$ ",	0 },
-	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS2=> ",	0 },
-	{ 0,	VSTRFIXED|VTEXTFIXED,		"PS4=+ ",	0 },
-	{ 0,	VSTRFIXED|VTEXTFIXED,		defoptindvar,	getoptsreset },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		defifsvar,	NULL },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAIL\0",	changemail },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAILPATH\0",	changemail },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		defpathvar,	changepath },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		"PS1=$ ",	NULL },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		"PS2=> ",	NULL },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		"PS4=+ ",	NULL },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		defoptindvar,	getoptsreset },
 #ifdef WITH_LINENO
-	{ 0,	VSTRFIXED|VTEXTFIXED,		linenovar,	0 },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED,		linenovar,	NULL },
 #endif
 #ifndef SMALL
-	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"TERM\0",	0 },
-	{ 0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"HISTSIZE\0",	sethistsize },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"TERM\0",	NULL },
+	{ NULL,	0,	VSTRFIXED|VTEXTFIXED|VUNSET,	"HISTSIZE\0",	sethistsize },
 #endif
 };
 
 STATIC struct var *vartab[VTABSIZE];
 
-STATIC struct var **hashvar(const char *);
 STATIC int vpcmp(const void *, const void *);
-STATIC struct var **findvar(struct var **, const char *);
+static struct var **findvar(const char *name, uint32_t *hash_out /* nullable */);
 
 /*
  * Initialize the varable symbol tables and import the environment
@@ -170,15 +170,19 @@ initvar(void)
 	vp = varinit;
 	end = vp + sizeof(varinit) / sizeof(varinit[0]);
 	do {
-		vpp = hashvar(vp->text);
+		uint32_t hash;
+		vpp = findvar(vp->text, &hash);
 		vp->next = *vpp;
+		vp->hash = hash;
 		*vpp = vp;
 	} while (++vp < end);
 	/*
 	 * PS1 depends on uid
 	 */
-	if (!geteuid())
+	if (!geteuid()) {
 		vps1.text = "PS1=# ";
+		vps1.hash = hash_var(vps1.text);
+	}
 }
 
 /*
@@ -246,10 +250,9 @@ intmax_t setvarint(const char *name, intmax_t val, int flags)
 struct var *setvareq(char *s, int flags)
 {
 	struct var *vp, **vpp;
-
-	vpp = hashvar(s);
+	uint32_t hash;
 	flags |= (VEXPORT & (((unsigned) (1 - aflag)) - 1));
-	vpp = findvar(vpp, s);
+	vpp = findvar(s, &hash);
 	vp = *vpp;
 	if (vp) {
 		if (vp->flags & VREADONLY) {
@@ -296,6 +299,7 @@ out_free:
 	if (!(flags & (VTEXTFIXED|VSTACK|VNOSAVE)))
 		s = savestr(s);
 	vp->text = s;
+	vp->hash = hash;
 	vp->flags = flags;
 
 out:
@@ -310,8 +314,7 @@ char *
 lookupvar(const char *name)
 {
 	struct var *v;
-
-	if ((v = *findvar(hashvar(name), name)) && !(v->flags & VUNSET)) {
+	if ((v = *findvar(name, NULL)) && !(v->flags & VUNSET)) {
 #ifdef WITH_LINENO
 		if (v == &vlineno && v->text == linenovar) {
 			fmtstr(linenovar+7, sizeof(linenovar)-7, "%d", lineno);
@@ -418,7 +421,7 @@ exportcmd(int argc, char **argv)
 			if ((p = strchr(name, '=')) != NULL) {
 				p++;
 			} else {
-				if ((vp = *findvar(hashvar(name), name))) {
+				if ((vp = *findvar(name, NULL))) {
 					vp->flags |= flag;
 					continue;
 				}
@@ -462,21 +465,23 @@ localcmd(int argc, char **argv)
 void mklocal(char *name, int flags)
 {
 	struct localvar *lvp;
-	struct var **vpp;
 	struct var *vp;
 
 	INTOFF;
 	lvp = ckmalloc(sizeof (struct localvar));
 	if (name[0] == '-' && name[1] == '\0') {
 		char *p;
+		uint32_t hash;
 		p = ckmalloc(sizeof(optlist));
 		lvp->text = memcpy(p, optlist, sizeof(optlist));
+		hash = hash_var(optlist);
+		lvp->hash = hash;
 		vp = NULL;
 	} else {
 		char *eq;
 
-		vpp = hashvar(name);
-		vp = *findvar(vpp, name);
+		uint32_t hash;
+		vp = *findvar(name, &hash);
 		eq = strchr(name, '=');
 		if (vp == NULL) {
 			if (eq)
@@ -487,6 +492,7 @@ void mklocal(char *name, int flags)
 		} else {
 			lvp->text = vp->text;
 			lvp->flags = vp->flags;
+			lvp->hash = vp->hash;
 			vp->flags |= VSTRFIXED|VTEXTFIXED;
 			if (eq)
 				setvareq(name, flags);
@@ -628,25 +634,13 @@ void unsetvar(const char *s)
 	setvar(s, 0, 0);
 }
 
-
-
 /*
- * Find the appropriate entry in the hash table from the name.
+ * Hashes the variable and returns a pointer to the var.
+ * hash_out, if not NULL, will contain name's hash.
+ * The hash function is the modified FNV1a with better avalanching.
+ * http://papa.bretmulvey.com/post/124027987928/hash-functions
  */
 
-STATIC struct var **
-hashvar(const char *p)
-{
-	unsigned int hashval;
-
-	hashval = ((unsigned char) *p) << 4;
-	while (*p && *p != '=')
-		hashval += (unsigned char) *p++;
-	return &vartab[hashval % VTABSIZE];
-}
-
-
-
 /*
  * Compares two strings up to the first = or '\0'.  The first
  * string must be terminated by '='; the second may be terminated by
@@ -678,13 +672,30 @@ vpcmp(const void *a, const void *b)
 	return varcmp(*(const char **)a, *(const char **)b);
 }
 
-STATIC struct var **
-findvar(struct var **vpp, const char *name)
+static struct var **findvar(const char *name, uint32_t *hash_out)
 {
+	struct var **vpp;
+	uint32_t hash = hash_var(name);
+	if (hash_out != NULL)
+		*hash_out = hash;
+
+	vpp = &vartab[hash % VTABSIZE];
+
 	for (; *vpp; vpp = &(*vpp)->next) {
-		if (varequal((*vpp)->text, name)) {
+#ifdef HASHTABLE_DEBUG
+		if (hash == (*vpp)->hash) {
+			if (varequal((*vpp)->text, name)) {
+				break;
+			} else {
+				TRACE(("%s collides with %s (hash 0x%08X)!!!\n",
+					(*vpp)->text, name, hash));
+			}
+		}
+#else
+		if (hash == (*vpp)->hash && varequal((*vpp)->text, name)) {
 			break;
 		}
+#endif
 	}
 	return vpp;
 }
diff --git a/src/var.h b/src/var.h
index cd0477f..4cc282f 100644
--- a/src/var.h
+++ b/src/var.h
@@ -54,6 +54,7 @@
 
 struct var {
 	struct var *next;		/* next entry in hash list */
+	uint32_t hash;			/* hash value for table collisions */
 	int flags;			/* flags are defined above */
 	const char *text;		/* name=value */
 	void (*func)(const char *);
@@ -65,6 +66,7 @@ struct var {
 struct localvar {
 	struct localvar *next;		/* next local variable in list */
 	struct var *vp;			/* the variable that was made local */
+	uint32_t hash;			/* hash value for table collisions */
 	int flags;			/* saved flags */
 	const char *text;		/* saved text */
 };
-- 
2.20.1

