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

sprintf() trigger buffer overflow on list_common() function #4

Open
datnlq opened this issue Oct 13, 2021 · 0 comments
Open

sprintf() trigger buffer overflow on list_common() function #4

datnlq opened this issue Oct 13, 2021 · 0 comments

Comments

@datnlq
Copy link

datnlq commented Oct 13, 2021

Hi @Gabe-commiter,
I found a issue, that can trigger buffer overflow on your application. The issue exists on list_common() function (from line 437 to 484) on ftpproto.c

By reading line by line, we can see you using sprintf() in your application. The problem appears when you defined buf and tmp variables both have 1024 length. Along with off's size = len(sprint()) so anytime you is using off += sprintf(), offset will be increased. Finally you get input from tmp to buf is trigger bufferoverflow.

void list_common(session_t *sess)
{
	DIR *dir = opendir(".");
	if (dir == NULL)
	{
		return;
	}

	struct dirent *dt;
	struct stat sbuf;
	while ((dt = readdir(dir)) != NULL)
	{
		if (lstat(dt->d_name, &sbuf) < 0)
		{
			continue;
		}
        if (dt->d_name[0] == '.')
		{
			continue;
        }

		char buf[1024] = {0}; //Defined buf's size is 1024

		const char *perms = statbuf_get_perms(&sbuf);

		int off = 0;
		off += sprintf(buf, "%s ", perms); // Define off size 
		off += sprintf(buf + off, " %3d %-8d %-8d ", (int)sbuf.st_nlink, sbuf.st_uid, sbuf.st_gid);// Define next write position to avoid overwriting
		off += sprintf(buf + off, "%8lu ", (unsigned long)sbuf.st_size);

		const char *datebuf = statbuf_get_date(&sbuf);
		off += sprintf(buf + off, "%s ", datebuf);
		if (S_ISLNK(sbuf.st_mode))
		{
			char tmp[1024] = {0}; // Defined tmp's size is 1024
			readlink(dt->d_name, tmp, sizeof(tmp)); //Get tmp with sizeof(tmp), max is 1024 from path in dt->d_name
			off += sprintf(buf + off, "%s -> %s\r\n", dt->d_name, tmp); // Read tmp to buf with max size is 1024, and buf max size is also 1024 so trigger bufferoverflow
		}
		else
		{
			off += sprintf(buf + off, "%s\r\n", dt->d_name);
		}

		writen(sess->data_fd, buf, strlen(buf));
	}

	closedir(dir);
}

Exploit:

sprintf() on line 473 read tmp and dt->name data in format defined before. But with data already written in from previous code, we can only write 1024 - off bytes. In case tmp have maximum size, input read into buf += 1024 + "\n\r" + leng(dt->name). It will trigger bufferoverflow.

Solution :
Please limit maximum input characters in snprintf().
Defined buff's maximum size is equal to the sum of the variables's size concatenated with the snprintf().

@datnlq datnlq changed the title Buffer Overflow sprintf() trigger buffer overflow on list_common() function Oct 13, 2021
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

1 participant