DEV Community

Roman Huang
Roman Huang

Posted on

I Audited a C Graph Algorithm Project and Found 13 Bugs — Including an Auth Bypass That Was Hiding in Plain Sight

The project

It's a campus tour guide system written in C. Console application, no GUI, no networking. A user logs in, picks from a menu, and queries information about 12 campus locations connected by a weighted undirected graph.

The algorithms involved:

  • Floyd-Warshall for all-pairs shortest paths (precomputed at startup)
  • DFS for finding all paths between two nodes
  • Recursive path reconstruction using a path[i][j] intermediate-node table
  • Adjacency matrix storage, graph built from a text file

Nothing exotic. Exactly the kind of project you'd see in a second-year data structures course. Which is why I decided to audit it carefully before publishing — because "basic" doesn't mean "correct," and the bugs in simple code are often the most instructive.

I found 13 issues. Here are the ones worth understanding.


Bug 1: The login function works. The caller doesn't.

This is my favorite find, because it's a category of bug that shows up in production systems too.

The validation function is correct:

int Login()
{
    char account[20]  = "NUC";
    char password[20] = "123";
    char account_input[20], password_input[20];

    printf("请输入账号:");
    scanf("%s", account_input);
    printf("请输入密码:");
    scanf("%s", password_input);

    if (strcmp(account, account_input) == 0 &&
        strcmp(password, password_input) == 0) {
        printf("登录成功!\n");
        return 1;          // success path: returns 1
    } else {
        printf("账号或密码有误\n");
    }
    Login();               // failure path: recurse, discard return value
}                          // no return on failure path
Enter fullscreen mode Exit fullscreen mode

It checks the credentials. It returns 1 on success. The problem is the caller:

case 1:
    system("cls");
    Login();       // return value: ignored
    Manager();     // always executes
    break;
Enter fullscreen mode Exit fullscreen mode

The return value of Login() is discarded. Manager() — the admin panel — runs regardless. You can type the wrong password and still get full admin access.

This is validation without verification. The check exists, the result exists, but the call site never acts on it. I see this pattern in real code when a validation function is added to an existing codebase but the call sites aren't updated to handle the new return value. The function does its job; the system doesn't.

There's a second problem in the same function: the failure branch recurses instead of looping. Every wrong password pushes a new stack frame. Enough failed attempts and you get a stack overflow. The fix is replacing the recursion with a loop that tracks attempt count.

// Fixed version
int Login(void)
{
    const char *account  = "NUC";
    const char *password = "123";
    char account_input[20], password_input[20];
    int attempts = 0;

    while (attempts < 3) {
        printf("请输入账号:");
        if (scanf("%19s", account_input) != 1) return 0;
        printf("请输入密码:");
        if (scanf("%19s", password_input) != 1) return 0;

        if (strcmp(account, account_input) == 0 &&
            strcmp(password, password_input) == 0) {
            printf("登录成功!\n");
            return 1;
        }
        attempts++;
        printf("账号或密码有误,还可尝试 %d 次\n", 3 - attempts);
    }
    return 0;
}
Enter fullscreen mode Exit fullscreen mode

And the caller actually checks the result:

case 1:
    system("cls");
    if (Login()) Manager();
    break;
Enter fullscreen mode Exit fullscreen mode

Bug 2: Buffer overflow that's hiding because of struct layout

The vertex struct stores location names:

typedef struct {
    int  num;
    char name[20];
    char intro[200];
} vertextype;
Enter fullscreen mode Exit fullscreen mode

The data file is read with:

fscanf(rf, "%d%s%s", &g->vexs[i].num, g->vexs[i].name, g->vexs[i].intro);
Enter fullscreen mode Exit fullscreen mode

No width specifier on %s. And one location name in the data file is "瑾瑜国际会议中心" — which in UTF-8 is 24 bytes, writing 5 bytes past the end of name[20].

The overflow overwrites the first 5 bytes of intro[200], the very next field. This doesn't crash because the overflow destination is allocated memory of the right type. It's undefined behavior that produces the "correct" output.

The interesting history: the data file appears to have originally been GBK-encoded. In GBK, "瑾瑜国际会议中心" is 16 bytes — it fits. At some point the encoding was converted to UTF-8, each Chinese character grew from 2 to 3 bytes, and the overflow was silently introduced.

The fix is expanding the buffer and adding a width specifier:

typedef struct {
    int  num;
    char name[64];     // 20 → 64
    char intro[512];   // 200 → 512
} vertextype;
Enter fullscreen mode Exit fullscreen mode
fscanf(rf, "%d%63s%511s", &g->vexs[i].num, g->vexs[i].name, g->vexs[i].intro);
Enter fullscreen mode Exit fullscreen mode

Bug 3: Sequence point UB that MSVC hides

printf("\n%s到%s的最短距离是:%dm\n",
    g->vexs[--sNum].name,
    g->vexs[--eNum].name,
    dist[sNum][eNum]);   // reads sNum and eNum after modifying them above
Enter fullscreen mode Exit fullscreen mode

GCC reports this clearly:

warning: operation on 'sNum' may be undefined [-Wsequence-point]
warning: operation on 'eNum' may be undefined [-Wsequence-point]
Enter fullscreen mode Exit fullscreen mode

MSVC evaluates function arguments right-to-left, so dist[sNum][eNum] gets evaluated before the decrements. The output looks correct. GCC may evaluate in a different order. The behavior is undefined — which means the compiler is within its rights to reorder, optimize, or do anything else.

The fix is two lines:

sNum--;
eNum--;
printf("\n%s到%s的最短距离是:%dm\n",
    g->vexs[sNum].name,
    g->vexs[eNum].name,
    dist[sNum][eNum]);
Enter fullscreen mode Exit fullscreen mode

Same logic, no UB.


The algorithm side: what's actually good here

The bugs above are real but fixable. The graph algorithm implementation is genuinely well-structured.

Floyd-Warshall with path reconstruction:

void ShortPath(mgraphtype *g) {
    int i, j, k;
    // initialize from adjacency matrix
    for (i = 0; i < g->vexNum; i++)
        for (j = 0; j < g->vexNum; j++) {
            dist[i][j] = g->edge[i][j];
            path[i][j] = (i != j && dist[i][j] < INFINITY) ? i : -1;
        }
    // three-pass relaxation
    for (k = 0; k < g->vexNum; k++)
        for (i = 0; i < g->vexNum; i++)
            for (j = 0; j < g->vexNum; j++)
                if (dist[i][j] > dist[i][k] + dist[k][j]) {
                    dist[i][j] = dist[i][k] + dist[k][j];
                    path[i][j] = k;
                }
}
Enter fullscreen mode Exit fullscreen mode

The path[i][j] table stores the last intermediate node on the optimal path from i to j. Path reconstruction is a recursive decomposition:

void Floyd_Print(mgraphtype *g, int sNum, int eNum) {
    if (path[sNum][eNum] == -1 ||
        path[sNum][eNum] == eNum ||
        path[sNum][eNum] == sNum) return;

    int mid = path[sNum][eNum];
    Floyd_Print(g, sNum, mid);
    printf("%s->", g->vexs[mid].name);
    Floyd_Print(g, mid, eNum);
}
Enter fullscreen mode Exit fullscreen mode

This is the same data structure idea as a next-hop routing table. For any pair (i,j), a single lookup gives you the relay point.

DFS all-paths with backtracking:

void Dfs_Print(mgraphtype *g, int sNum, int eNum) {
    pathStack[top++] = sNum;
    visited[sNum] = 1;

    for (int i = 0; i < g->vexNum; i++) {
        if (g->edge[sNum][i] > 0 && g->edge[sNum][i] != INFINITY && !visited[i]) {
            if (i == eNum) {
                // print current path + edge to destination
            } else {
                Dfs_Print(g, i, eNum);
                top--;
                visited[i] = 0;    // backtrack
            }
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

Clean backtracking: visited[i] = 0 when popping ensures each branch is explored independently. The path length accumulation has a small quirk (the last edge calculated separately) but the logic is correct.


What the full audit found

13 issues total across 5 severity levels. The interesting ones:

  • Build chain broken in 3 places (.sln.vcxproj.c references all wrong after a rename) — the project can't be opened in Visual Studio as-is
  • fscanf returns 18 iterations but the file only has 16 edges — the last two iterations silently duplicate the final edge
  • fopen("file", "r") followed by fprintf — the announcement bulletin feature writes nothing but reports success
  • Judge_Input hardcodes the upper limit as 12 instead of reading g->vexNum
  • fclose(NULL) when fopen fails — undefined behavior

Full write-up with evidence, compiler output, and suggested fixes for all 13:
TECHNICAL_ISSUES.md


Lessons

Check return values. Not just malloc and fopen. scanf, fprintf, comparison functions, your own functions. The auth bypass exists entirely because one call site ignores one return value.

Test with all target encodings. If your data file can ever be UTF-8 and your buffers were sized for GBK, the math changes. A 20-byte buffer that fits 10 GBK characters holds only 6 complete UTF-8 Chinese characters.

-Wall -Wextra before you ship. The sequence point bug is a textbook -Wsequence-point warning. The fclose(NULL) path triggers with a missing file. The recursive non-void function end is -Wreturn-type. GCC found most of the interesting bugs with flags alone.

Undefined behavior that works is still undefined behavior. The buffer overflow doesn't crash because the overflow lands in the next struct field. The sequence point UB gives the right answer on MSVC because of evaluation order. Neither is stable. Neither is safe to ship.

Top comments (0)