[MPlayer-dev-eng] [PATCH 1/4] stream ftp: Pass full buffer size to snprintf

Alexander Strasser eclipse7 at gmx.net
Thu Nov 22 22:22:50 CET 2012


Previously the buffer size was always passed as one less than
the underlying buffer's size. This is not using the underlying
buffer to its full potential according to the C99 standard. The
last byte of the buffers were never used.

No vulnerabilities should have been caused by this mistake because
the strings stored in the buffers were zero terminated at all
times. Neither were out-of-array writes nor reads possible.

Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 stream/stream_ftp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/stream/stream_ftp.c b/stream/stream_ftp.c
index 4d0925d..372051a 100644
--- a/stream/stream_ftp.c
+++ b/stream/stream_ftp.c
@@ -281,7 +281,7 @@ static int FtpOpenPort(struct stream_priv_s* p) {
 
   sscanf(par+1,"%u,%u,%u,%u,%u,%u",&num[0],&num[1],&num[2],
 	 &num[3],&num[4],&num[5]);
-  snprintf(str,127,"%d.%d.%d.%d",num[0],num[1],num[2],num[3]);
+  snprintf(str,sizeof(str),"%d.%d.%d.%d",num[0],num[1],num[2],num[3]);
   fd = connect2Server(str,(num[4]<<8)+num[5],0);
 
   if(fd < 0)
@@ -301,7 +301,7 @@ static int FtpOpenData(stream_t* s,off_t newpos) {
   if(s->fd < 0) return 0;
 
   if(newpos > 0) {
-    snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"REST %"PRId64, (int64_t)newpos);
+    snprintf(p->cmd_buf,CMD_BUFSIZE,"REST %"PRId64, (int64_t)newpos);
 
     resp = FtpSendCmd(p->cmd_buf,p,rsp_txt);
     if(resp != 3) {
@@ -311,7 +311,7 @@ static int FtpOpenData(stream_t* s,off_t newpos) {
   }
 
   // Get the file
-  snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"RETR %s",p->filename);
+  snprintf(p->cmd_buf,CMD_BUFSIZE,"RETR %s",p->filename);
   resp = FtpSendCmd(p->cmd_buf,p,rsp_txt);
 
   if(resp != 1) {
@@ -464,12 +464,12 @@ static int open_f(stream_t *stream,int mode, void* opts, int* file_format) {
   }
 
   // Login
-  snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"USER %s",p->user);
+  snprintf(p->cmd_buf,CMD_BUFSIZE,"USER %s",p->user);
   resp = FtpSendCmd(p->cmd_buf,p,rsp_txt);
 
   // password needed
   if(resp == 3) {
-    snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"PASS %s",p->pass);
+    snprintf(p->cmd_buf,CMD_BUFSIZE,"PASS %s",p->pass);
     resp = FtpSendCmd(p->cmd_buf,p,rsp_txt);
     if(resp != 2) {
       mp_msg(MSGT_OPEN,MSGL_ERR, "[ftp] command '%s' failed: %s\n",p->cmd_buf,rsp_txt);
@@ -491,7 +491,7 @@ static int open_f(stream_t *stream,int mode, void* opts, int* file_format) {
   }
 
   // Get the filesize
-  snprintf(p->cmd_buf,CMD_BUFSIZE - 1,"SIZE %s",p->filename);
+  snprintf(p->cmd_buf,CMD_BUFSIZE,"SIZE %s",p->filename);
   resp = FtpSendCmd(p->cmd_buf,p,rsp_txt);
   if(resp != 2) {
     mp_msg(MSGT_OPEN,MSGL_WARN, "[ftp] command '%s' failed: %s\n",p->cmd_buf,rsp_txt);
-- 
1.7.10.2.552.gaa3bb87


More information about the MPlayer-dev-eng mailing list