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 do_retr() function #2

Open
H4niz opened this issue Aug 24, 2021 · 0 comments
Open

sprintf() trigger buffer overflow on do_retr() function #2

H4niz opened this issue Aug 24, 2021 · 0 comments

Comments

@H4niz
Copy link

H4niz commented Aug 24, 2021

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

At glance, we can see you defined char arg[MAX_ARG]; , it's not problem, however, when you use sprintf() on line 718 and 721, they trigger bufferoverflow.

typedef struct session
{
/*
	Defined on common.h
		#define MAX_COMMAND_LINE 1024
		#define MAX_COMMAND 32
		#define MAX_ARG     1024

		#define MAX_BUFFER_SIZE 1024
*/

#define MAX_BUFFER_SIZE 1024
	uid_t    uid;
	int ctrl_fd;//¿ØÖÆÃèÊö·û
	char cmdline[MAX_COMMAND_LINE];
	char cmd[MAX_COMMAND];
	char arg[MAX_ARG]; 
....
	<truncated>
....
}session_t;
static void do_retr(session_t *sess)
{
	<truncated>
....
	char buf[MAX_BUFFER_SIZE] = {0}; // Defined buffer called buf with MAX_BUFFER_SIZE length
	//2Åжϴ«Êäģʽ
	if(sess->is_ascii)
// trigger the buffer overflow because length of sess->arg is defined 1024 length, 
// in order that, when sprintf is executed, the buffer `buf` can be write total  len("Opening ASCII mode data connection for ") + MAX_BUFFER_SIZE + len(" (%ld bytes)")
		sprintf(buf,  "Opening ASCII mode data connection for %s (%ld bytes)", sess->arg, sbuf.st_size);//Ascii
	else
// trigger the buffer overflow because length of sess->arg is defined 1024 length, 
// in order that, when sprintf is executed, the buffer `buf` can be write total  len("Opening ASCII mode data connection for ") + MAX_BUFFER_SIZE + len(" (%ld bytes)")
		sprintf(buf, "Opening BINARY mode data connection for %s (%ld bytes)", sess->arg, sbuf.st_size);
...
	<truncated>
...
}

Solution: Please use snprintf() to limit maximum input characters.
See: https://www.geeksforgeeks.org/snprintf-c-library/

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