Removed fixed-length buffer in is_local() by rewriting it to use a
state machine. Characters from the passwd wfile are just compared
against characters in the ueername directly. Added a set of test
cases, made the makefile run these for "make check". Since is_local
does not modify the characters pointed to by its argument, we also
declare them to be const.
---
login-utils/Makefile.am | 7 ++-
login-utils/islocal.c | 164 +++++++++++++++++++++++++++++++++++++++++-----
login-utils/islocal.h | 2 +-
3 files changed, 153 insertions(+), 20 deletions(-)
diff --git a/login-utils/Makefile.am b/login-utils/Makefile.am
index 8f5dc89..7c9a7a3 100644
--- a/login-utils/Makefile.am
+++ b/login-utils/Makefile.am
@@ -130,7 +130,12 @@ install-exec-hook::
endif
-noinst_PROGRAMS = checktty_test
+noinst_PROGRAMS = checktty_test islocal_test
checktty_test_SOURCES = checktty.c login.h
checktty_test_CPPFLAGS = -DMAIN_TEST_CHECKTTY $(AM_CPPFLAGS)
+islocal_test_SOURCES = islocal.c
+islocal_test_CPPFLAGS = -DMAIN_TEST_ISLOCAL $(AM_CPPFLAGS)
+check-local: islocal_test$(EXEEXT) checktty_test$(EXEEXT)
+ ./islocal_test$(EXEEXT)
+ ./checktty_test$(EXEEXT)
diff --git a/login-utils/islocal.c b/login-utils/islocal.c
index 2976980..98c2089 100644
--- a/login-utils/islocal.c
+++ b/login-utils/islocal.c
@@ -12,7 +12,10 @@
1999-02-22 Arkadiusz Mi¶kiewicz <misiek@xxxxxxxxxx>
- added Native Language Support
-
+
+ 2008-04-06 James Youngman, jay@xxxxxxx
+ - Completely rewritten to remove assumption that /etc/passwd
+ lines are < 1024 characters long. Also added unit tests.
*/
@@ -24,29 +27,154 @@
#include "pathnames.h"
#include "islocal.h"
-#define MAX_LENGTH 1024
+#if MAIN_TEST_ISLOCAL
+static const char *testdata;
+static FILE *
+fopen_passwd_file(void) {
+ FILE *f = tmpfile();
+ if (NULL == f) {
+ perror("Failed to create temporary file for test, exiting.");
+ exit(1);
+ }
+ if (fprintf(f, "%s", testdata) < 0) {
+ perror("Failed to write to temporary file for test, exiting.");
+ fclose(f);
+ exit(1);
+ }
+ /* don't use rewind(), it has no error reporting. */
+ if (fseek(f, 0L, SEEK_SET) < 0) {
+ perror("temporary file");
+ fclose(f);
+ exit(1);
+ }
+ return f;
+}
+#else
+static FILE *
+fopen_passwd_file(void) {
+ FILE *f = fopen(_PATH_PASSWD, "r");
+ if (NULL == f) {
+ perror(_PATH_PASSWD);
+ fprintf(stderr, _("Failed to read %s, exiting."), _PATH_PASSWD);
+ exit(1);
+ }
+ return f;
+}
+#endif
+
int
-is_local(char *user)
+is_local(const char *user)
{
- FILE *fd;
- char line[MAX_LENGTH];
int local = 0;
- size_t len;
-
- if(!(fd = fopen(_PATH_PASSWD, "r"))) {
- fprintf(stderr,_("Can't read %s, exiting."),_PATH_PASSWD);
- exit(1);
- }
+ size_t match;
+ int chin, skip;
+ FILE *f = fopen_passwd_file();
- len = strlen(user);
- while(fgets(line, MAX_LENGTH, fd)) {
- if(!strncmp(line, user, len) && line[len] == ':') {
- local = 1;
- break;
- }
+ match = 0u;
+ skip = 0;
+ while ((chin=getc(f)) != EOF) {
+ if (skip) {
+ /* Looking for the start of the next line. */
+ if ('\n' == chin) {
+ /* Start matching username at the next char. */
+ skip = 0;
+ match = 0u;
+ }
+ } else {
+ if (':' == chin) {
+ if (0 == user[match]) {
+ local = 1; /* Success. */
+ /* next line has no test coverage, but it is
+ * just an optimisation anyway. */
+ break;
+ } else {
+ /* we read a whole username, but it is
+ * the wrong user. Skip to the next
+ * line. */
+ skip = 1;
+ }
+ } else if ('\n' == chin) {
+ /* This line contains no colon; it's malformed.
+ * No skip since we are already at the start of
+ * the next line. */
+ match = 0u;
+ } else if (chin != user[match]) {
+ /* username does not match. */
+ skip = 1;
+ } else {
+ ++match;
+ }
+ }
}
- fclose(fd);
+ fclose(f);
return local;
}
+#if MAIN_TEST_ISLOCAL
+static unsigned failures = 0u;
+static const char *testdata =
+"root:x:0:0:root:/root:/bin/bash\n"
+"sys:x:3:3:sys:/dev:/bin/sh\n"
+"orac:x:33:33:sys:/dev:/bin/youngman\n"
+/* Next line should be >1024 but <2048 */
+"long:x:4:4:foo:/home/loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonnnggg:/bin/sh\n"
+"al:x:5:5:fnord:/dev:/bin/sh\n"
+"abcdefgh:x:6:3:fnord:/dev:/bin/sh\n"
+"malformed\n"
+"\n" /* see test case znobody */
+"nobody:x:65534:65534:nobody:/nonexistent:/bin/sh\n"
+"nonl:x:65532:65532:no newline:/:/bin/false";
+
+struct testcase {
+ int expected;
+ const char *name;
+};
+#define NUM_TESTS (sizeof(tests)/sizeof(tests[0]))
+
+const struct testcase tests[] = {
+ { 1, "root" }, /* first */
+ { 1, "nobody" }, /* last */
+ { 0, "" }, /* empty */
+ { 0, "youngman" }, /* 8 chars, is suffix of another entry (the shell of user orac) */
+ { 0, "youngman2" }, /* 9 chars */
+ { 0, "abcdefghx" }, /* verifies first skip */
+ { 0, "nobo" }, /* prefix of existing entry */
+ { 1, "long" }, /* very long entry */
+ { 0, "rot", }, /* verifies that we skip the rest of the line when we see a mismatch */
+ { 1, "al" }, /* immediately after long entry */
+ { 0, "malformed" }, /* exists in field 0 but the line is malformed */
+ { 1, "nonl" }, /* I guess you could argue this either way */
+ { 0, "znobody" }, /* verify that after malformed line, we restart match at 0 (needs the empty line) */
+};
+
+static int
+fail (int i) {
+ printf("FAIL: [%s], expected %s\n",
+ tests[i].name,
+ (tests[i].expected ? "local" : "not local"));
+ ++failures;
+}
+
+static int
+pass (int i) {
+ printf("PASS: [%s]\n", tests[i].name);
+}
+
+int
+main (int argc, char *argv[]) {
+ size_t i;
+ const char *me = "islocal_test";
+ for (i=0u; i<NUM_TESTS; i++) {
+ const int got = is_local(tests[i].name) ? 1 : 0;
+ ((tests[i].expected == got) ? pass : fail)(i);
+ }
+ if (!failures) {
+ printf ("%s: All tests passed.\n", me);
+ return 0;
+ } else {
+ printf ("%s: %u tests FAILED.\n", me, failures);
+ return 1;
+ }
+}
+#endif
diff --git a/login-utils/islocal.h b/login-utils/islocal.h
index 305bc57..2c58799 100644
--- a/login-utils/islocal.h
+++ b/login-utils/islocal.h
@@ -1 +1 @@
-extern int is_local(char *user);
+extern int is_local(const char *user);
--
1.5.3.8
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
