Results of code review

doc/2010-10-NMT/chase.svg

@@ -42,8 +42,8 @@
      inkscape:document-units="px"
      inkscape:current-layer="layer1"
      showgrid="false"
-     inkscape:window-width="704"
+     inkscape:window-width="1276"
-     inkscape:window-height="1010"
+     inkscape:window-height="1006"
      inkscape:window-x="0"
      inkscape:window-y="14" />
   <metadata
@@ -322,7 +322,7 @@
          sodipodi:role="line"
          x="381.88013"
          y="787.68048"
-         id="tspan2823">and NMT ACS chapter</tspan><tspan
+         id="tspan2823">and NMT ACM chapter</tspan><tspan
          sodipodi:role="line"
          x="381.88013"
          y="811.68048"

doc/2010-10-NMT/pacman2.svg

@@ -17,7 +17,15 @@
    inkscape:output_extension="org.inkscape.output.svg.inkscape"
    version="1.0">
   <defs
-     id="defs2569" />
+     id="defs2569">
+    <inkscape:perspective
+       sodipodi:type="inkscape:persp3d"
+       inkscape:vp_x="0 : 495 : 1"
+       inkscape:vp_y="0 : 1000 : 0"
+       inkscape:vp_z="765 : 495 : 1"
+       inkscape:persp3d-origin="382.5 : 330 : 1"
+       id="perspective2545" />
+  </defs>
   <sodipodi:namedview
      id="base"
      pagecolor="#ffffff"
@@ -703,7 +711,7 @@
          sodipodi:role="line"
          x="381.88013"
          y="787.68048"
-         id="tspan2823">and NMT ACS chapter</tspan><tspan
+         id="tspan2823">and NMT ACM chapter</tspan><tspan
          sodipodi:role="line"
          x="381.88013"
          y="811.68048"

doc/2010-10-NMT/ship.svg

@@ -70,8 +70,8 @@
      inkscape:document-units="px"
      inkscape:current-layer="layer1"
      showgrid="false"
-     inkscape:window-width="704"
+     inkscape:window-width="1276"
-     inkscape:window-height="1010"
+     inkscape:window-height="1006"
      inkscape:window-x="0"
      inkscape:window-y="14"
      showguides="true"
@@ -119,7 +119,7 @@
          sodipodi:role="line"
          x="381.88013"
          y="787.68048"
-         id="tspan2823">and NMT ACS chapter</tspan><tspan
+         id="tspan2823">and NMT ACM chapter</tspan><tspan
          sodipodi:role="line"
          x="381.88013"
          y="811.68048"

doc/packages.txt

@@ -0,0 +1,49 @@
+CTF Packages
+============
+
+Packages are squashfs files.  
+
+A hypothetical package named pkgname.sfs will be mounted under
+/srv/pkgname.  The following top-level files and directories are
+significant:
+
+* /www/         -  Will appear as http://host/pkgname/, CGI can be run
+* /bin/         -  Will be added to $PATH
+* /puzzles/     -  Will appear as a puzzle category (see "Puzzles" below)
+* /answers.txt  -  Puzzle answers for category pkgname (see "Puzzles" below)
+
+
+Puzzles
+-------
+
+To expose puzzles, place them in the /puzzles/ directory, like so:
+
+    /puzzles/10/index.html
+    /puzzles/20/index.html
+    /puzzles/20/script.cgi
+    /puzzles/30/index.html
+    /puzzles/30/something.jpg
+    /puzzles/40/index.html
+
+where the second directory is the point value of the puzzle.  This means
+that no two puzzles in a category can have the same point value.
+
+Files will be served up from the web server, and CGI scripts will be
+executed.
+
+Store answers to your puzzles in /answers.txt.  Answers are one per
+line, with the point value appearing first, followed by a space, then
+the answer.  Answers are case sensitive.  You may have multiple answers
+for each point value.
+
+
+    10 zip file
+    10 zip
+    10 ZIP
+    10 pkzip
+    10 PKZIP
+    20 varname
+    30 JFIF
+    40 0x8040fe67
+    40 8040FE67
+    40 8040fe67

src/Makefile

@@ -1,3 +1,4 @@
+CFLAGS = -Wall
 TARGETS = in.tokend pointscli claim.cgi puzzler.cgi puzzles.cgi
 
 all: build

src/claim.cgi.c

@@ -4,11 +4,8 @@
 int
 main(int argc, char *argv[])
 {
-  char   team[9];
+  char   team[TEAM_MAX]   = {0};
-  char   token[100];
+  char   token[TOKEN_MAX] = {0};
-
-  team[0]  = 0;
-  token[0] = 0;
 
   if (-1 == cgi_init(argv)) {
     return 0;
@@ -65,9 +62,14 @@ main(int argc, char *argv[])
     }
     category[i] = '\0';
 
-    award_and_log_uniquely(team, category, 1,
+    {
-                           "tokens.db",
+      char line[TEAM_MAX + TOKEN_MAX + 1];
+
+      my_snprintf(line, sizeof(line),
                   "%s %s", team, token);
+      award_and_log_uniquely(team, category, 1,
+                             "tokens.db", line);
+    }
   }
 
 

src/common.c

@@ -10,6 +10,21 @@
 #include <time.h>
 #include "common.h"
 
+#ifdef NODUMP
+#  define DUMPf(fmt, args...)
+#else
+#  define DUMPf(fmt, args...) fprintf(stderr, "%s:%s:%d " fmt "\n", __FILE__, __FUNCTION__, __LINE__, ##args)
+#endif
+#define DUMP() DUMPf("")
+#define DUMP_d(v) DUMPf("%s = %d", #v, v)
+#define DUMP_x(v) DUMPf("%s = 0x%x", #v, v)
+#define DUMP_s(v) DUMPf("%s = %s", #v, v)
+#define DUMP_c(v) DUMPf("%s = '%c' (0x%02x)", #v, v, v)
+#define DUMP_p(v) DUMPf("%s = %p", #v, v)
+
+
+#define POST_MAX 1024
+
 /*
  * CGI
  */
@@ -51,6 +66,9 @@ read_char_stdin()
     char *p = getenv("CONTENT_LENGTH");
     if (p) {
       inlen = atoi(p);
+      if (inlen > POST_MAX) {
+        inlen = POST_MAX;
+      }
     } else {
       inlen = 0;
     }
@@ -224,7 +242,7 @@ cgi_error(char *fmt, ...)
  */
 
 
-#define EOL(c) ((EOF == (c)) || (0 == (c)) || ('\n' == (c)))
+#define EOL(c) ((EOF == (c)) || ('\n' == (c)))
 
 int
 fgrepx(char const *needle, char const *filename)
@@ -240,19 +258,20 @@ fgrepx(char const *needle, char const *filename)
 
       /* This list of cases would have looked so much nicer in OCaml.  I
          apologize. */
-      if (EOL(c) && (0 == *p)) {
+      if (EOL(c) && ('\0' == *p)) {
         found = 1;
         break;
-      } else if (EOF == c) {
+      } else if (EOF == c) {    /* End of file */
         break;
-      } else if ((0 == p) || (*p != c)) {
+      } else if (('\0' == p) || (*p != c)) {
         p = needle;
+        /* Discard the rest of the line */
         do {
           c = fgetc(f);
         } while (! EOL(c));
-      } else if ('\n' == c) {
+      } else if (EOL(c)) {
         p = needle;
-      } else {
+      } else {                  /* It matched */
         p += 1;
       }
     }
@@ -271,7 +290,7 @@ my_snprintf(char *buf, size_t buflen, char *fmt, ...)
   va_start(ap, fmt);
   len = vsnprintf(buf, buflen - 1, fmt, ap);
   va_end(ap);
-  buf[buflen] = '\0';
+  buf[buflen - 1] = '\0';
   if (len >= buflen) {
     return buflen - 1;
   } else {
@@ -320,8 +339,8 @@ team_exists(char const *teamhash)
     }
   }
 
-  /* lstat seems to be the preferred way to check for existence. */
+  /* stat seems to be the preferred way to check for existence. */
-  ret = lstat(srv_path("teams/names/%s", teamhash), &buf);
+  ret = stat(srv_path("teams/names/%s", teamhash), &buf);
   if (-1 == ret) {
     return 0;
   }
@@ -396,27 +415,23 @@ award_points(char const *teamhash,
   return 0;
 }
 
+/** Award points iff they haven't been logged.
+
+    If [line] is not in [dbfile], append it and give [points] to [team]
+    in [category].
+*/
 void
 award_and_log_uniquely(char const *team,
                        char const *category,
                        long points,
                        char const *dbfile,
-                       char const *fmt, ...)
+                       char const *line)
 {
   char    *dbpath = srv_path(dbfile);
-  char     line[200];
-  int      len;
   int      ret;
   int      fd;
-  va_list  ap;
 
   /* Make sure they haven't already claimed these points */
-  va_start(ap, fmt);
-  len = vsnprintf(line, sizeof(line), fmt, ap);
-  va_end(ap);
-  if (sizeof(line) <= len) {
-    cgi_error("Log line too long");
-  }
   if (fgrepx(line, dbpath)) {
     cgi_page("Already claimed",
              "<p>Your team has already claimed these points.</p>");
@@ -437,11 +452,11 @@ award_and_log_uniquely(char const *team,
   }
 
   /* Log that we did so */
-  /* We can turn that trailing NUL into a newline now since write
-     doesn't use C strings */
-  line[len] = '\n';
   lseek(fd, 0, SEEK_END);
-  if (-1 == write(fd, line, len+1)) {
+  if (-1 == write(fd, line, strlen(line))) {
+    cgi_error("Unable to append log");
+  }
+  if (-1 == write(fd, "\n", 1)) {
     cgi_error("Unable to append log");
   }
   close(fd);

src/common.h

@@ -5,6 +5,7 @@
 
 #define TEAM_MAX 40
 #define CAT_MAX 40
+#define TOKEN_MAX 40
 
 int cgi_init(char *global_argv[]);
 size_t cgi_item(char *str, size_t maxlen);
@@ -24,6 +25,6 @@ void award_and_log_uniquely(char const *team,
                             char const *category,
                             long points,
                             char const *logfile,
-                            char const *fmt, ...);
+                            char const *line);
 
 #endif

src/in.tokend.c

@@ -120,7 +120,7 @@ main(int argc, char *argv[])
     bubblebabble(digest, crap, itokenlen);
 
     /* Append digest to service name. */
-    tokenlen = (size_t)snprintf(token, sizeof(token),
+    tokenlen = (size_t)my_snprintf(token, sizeof(token),
                                    "%s:%s",
                                    service, digest);
   }
@@ -131,7 +131,7 @@ main(int argc, char *argv[])
     int ret;
 
     do {
-      fd = open(srv_path("tokens.db"), O_WRONLY | O_CREAT, 0644);
+      fd = open(srv_path("tokens.db"), O_WRONLY | O_CREAT, 0666);
       if (-1 == fd) break;
 
       ret = lockf(fd, F_LOCK, 0);
@@ -150,16 +150,18 @@ main(int argc, char *argv[])
       if (-1 == ret) break;
     } while (0);
 
-    if (-1 == ret) {
+    if ((-1 == fd) || (-1 == ret)) {
       printf("!%s", strerror(errno));
       return 0;
     }
   }
 
   /* Encrypt the token.  Note that now tokenlen is in uint32_ts, not
-     chars! */
+     chars!  Also remember that token must be big enough to hold a
+     multiple of 4 chars, since tea will go ahead and jumble them up for
+     you.  If the compiler aligns words this shouldn't be a problem. */
   {
-    tokenlen = (tokenlen + (tokenlen % 4)) / 4;
+    tokenlen = (tokenlen + (tokenlen % sizeof(uint32_t))) / sizeof(uint32_t);
 
     tea_encode(key, (uint32_t *)token, tokenlen);
   }

src/puzzler.cgi.c

@@ -1,20 +1,15 @@
 #include <stdlib.h>
 #include "common.h"
 
-
 int
 main(int argc, char *argv[])
 {
-  char team[TEAM_MAX];
+  char team[TEAM_MAX]    = {0};
-  char category[CAT_MAX];
+  char category[CAT_MAX] = {0};
-  char points_str[5];
+  char points_str[11]    = {0};
-  char answer[500];
+  char answer[500]       = {0};
   long points            = 0;
 
-  team[0]     = 0;
-  category[0] = 0;
-  answer[0]   = 0;
-
   if (-1 == cgi_init(argv)) {
     return 0;
   }
@@ -71,9 +66,14 @@ main(int argc, char *argv[])
     }
   }
 
-  award_and_log_uniquely(team, category, points,
+  {
-                         "puzzler.db",
+    char line[TEAM_MAX + CAT_MAX + sizeof(points_str) + 2];
+
+    my_snprintf(line, sizeof(line),
                "%s %s %ld", team, category, points);
+    award_and_log_uniquely(team, category, points,
+                           "puzzler.db", line);
+  }
 
   cgi_page("Points awarded",
            ("<p>%d points for %s.</p>"

src/puzzles.cgi.c

@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include "common.h"
 
+
 int
 longcmp(long *a, long *b)
 {
@@ -15,13 +16,13 @@ longcmp(long *a, long *b)
   return 0;
 }
 
-#define PUZZLES_MAX 500
+#define PUZZLES_MAX 100
 
 /** Keeps track of the most points yet awarded in each category */
 struct {
   char cat[CAT_MAX];
   long points;
-} points_by_cat[100];
+} points_by_cat[PUZZLES_MAX];
 int ncats = 0;
 
 void
@@ -37,14 +38,19 @@ read_points_by_cat()
   }
 
   while (1) {
-    if (2 != fscanf(f, "%*s %s %ld\n", &cat, &points)) {
+    /* XXX: tokenize like cgi_item */
+    if (2 != fscanf(f, "%*s %s %ld\n", cat, &points)) {
       break;
     }
     for (i = 0; i < ncats; i += 1) {
       if (0 == strcmp(cat, points_by_cat[i].cat)) break;
     }
     if (i == ncats) {
-      strcpy(points_by_cat[i].cat, cat);
+      if (PUZZLES_MAX == ncats) {
+        continue;
+      }
+      strncpy(points_by_cat[i].cat, cat, sizeof(points_by_cat[i].cat));
+      points_by_cat[i].points = 0;
       ncats += 1;
     }
     if (points > points_by_cat[i].points) {
@@ -76,12 +82,14 @@ main(int argc, char *argv[])
   /* For each file in /srv/ ... */
   while (1) {
     struct dirent *e          = readdir(srv);
-    char          *cat        = e->d_name;
+    char          *cat;
     DIR           *puzzles;
     long           catpoints[PUZZLES_MAX];
     size_t         ncatpoints = 0;
 
     if (! e) break;
+
+    cat = e->d_name;
     if ('.' == cat[0]) continue;
     /* We have to lstat anyway to see if it's a directory; may as
        well just barge ahead and watch for errors. */