Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Found bug, please accept a patch in upstream #243

Open
nevapadonak opened this issue Aug 12, 2022 · 1 comment
Open

Found bug, please accept a patch in upstream #243

nevapadonak opened this issue Aug 12, 2022 · 1 comment

Comments

@nevapadonak
Copy link

Hello,
The bug is in xl2tpd-control.c, static array commands has no last element filled by NULLs:

struct command_t commands[] = {
    {"add-lac",       &command_add_lac,       TUNNEL_REQUIRED},
    {"connect-lac",   &command_connect_lac,   TUNNEL_REQUIRED},
    {"disconnect-lac",&command_disconnect_lac,TUNNEL_REQUIRED},
    {"status-lac",    &command_status_lac,    TUNNEL_REQUIRED},
    {"remove-lac",    &command_remove_lac,    TUNNEL_REQUIRED},
    {"available",     &command_available,     TUNNEL_NOT_REQUIRED},
    {"add-lns",       &command_add_lns,       TUNNEL_REQUIRED},
    {"status-lns",    &command_status_lns,    TUNNEL_REQUIRED},
    {"remove-lns",    &command_remove_lns,    TUNNEL_REQUIRED},
};

There is the loop in main() going through this array:

    /* parse command name */
    for (command = commands; command->name; command++)
    {
        if (!strcasecmp (argv[i], command->name))
        {
            i++;
            break;
        }
    }

But there is no guarantee that after the last element the memory will be filled by zeroes.
So it may works and may not.
In my environment (ARM, OpenWRT v21.02.0 distro) the program fails with segmentation fault.

This is the fix below. Please add in your code.

--- a/xl2tpd-control.c	2022-08-11 21:39:47.754446499 +0300
+++ b/xl2tpd-control.c	2022-08-11 21:40:16.849022167 +0300
@@ -83,7 +83,9 @@
     {"available",     &command_available,     TUNNEL_NOT_REQUIRED},
     {"add-lns",       &command_add_lns,       TUNNEL_REQUIRED},
     {"status-lns",    &command_status_lns,    TUNNEL_REQUIRED},
-    {"remove-lns",    &command_remove_lns,    TUNNEL_REQUIRED}
+    {"remove-lns",    &command_remove_lns,    TUNNEL_REQUIRED},
+
+    {NULL, NULL, TUNNEL_REQUIRED}
 };
 
 void usage()
nevapadonak pushed a commit to nevapadonak/xl2tpd that referenced this issue Aug 12, 2022
nevapadonak pushed a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak pushed a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak pushed a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak pushed a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak added a commit to nevapadonak/xl2tpd that referenced this issue Aug 12, 2022
nevapadonak added a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak added a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak added a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
nevapadonak added a commit to nevapadonak/packages that referenced this issue Aug 12, 2022
@shussain
Copy link
Collaborator

PR merged into 1.3.19dev branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants