CVE-2022-24792: Loop with Unreachable Exit Condition ('Infinite Loop') (CWE-835)
value: PJSIP is a free and open source multimedia communication library written in C. A denial-of-service vulnerability affects applications on a 32-bit systems that use PJSIP versions 2.12 and prior to play/read invalid WAV files. The vulnerability occurs when reading WAV file data chunks with length greater than 31-bit integers. The vulnerability does not affect 64-bit apps and should not affect apps that only plays trusted WAV files. A patch is available on the `master` branch of the `pjsip/project` GitHub repository. As a workaround, apps can reject a WAV file received from an unknown source or validate the file first.
CWE description: The product contains an iteration or loop with an exit condition that cannot be reached, i.e., an infinite loop.
CVSS v3 base score: 7.5
Published: 2022-04-25T16:16Z
Repository: https://github.com/pjsip/pjproject
Language: C
Files changed: pjlib/include/pj/types.h, pjlib/src/pj/file_io_ansi.c, pjmedia/src/pjmedia/avi_player.c, pjmedia/src/pjmedia/wav_player.c, pjmedia/src/pjmedia/wav_playlist.c
Commit date: 2022-04-25 12:54:19 +0800
Commit message: Merge pull request from GHSA-rwgw-vwxg-q799

* Prevent potential infinite loop when parsing WAV format file

* Check if subchunk is negative.

* Fix and add checks

* Change data type from pj_ssize_t to long.

* Modify check

* Fix leak file descriptor and modify check on wav_playlist

* Move overflow/underflow check to pj_file_setpos()

* Use macro to simplify check

* modification based on comments

* Remove unnecessary casting

* Modification based on comments
Diff: diff --git a/pjlib/include/pj/types.h b/pjlib/include/pj/types.h
index 8c9f78238f..e9a60758c0 100644
--- a/pjlib/include/pj/types.h
+++ b/pjlib/include/pj/types.h
@@ -29,10 +29,11 @@
  * @defgroup PJ_BASIC Basic Data Types and Library Functionality.
  * @ingroup PJ_DS
  * @{
  */
 #include <pj/config.h>
+#include <pj/limits.h>
 
 PJ_BEGIN_DECL
 
 /* ************************************************************************* */
 
@@ -359,10 +360,17 @@ PJ_INLINE(pj_int32_t) pj_swap32(pj_int32_t val32)
     *(p+1) = *(p+2);
     *(p+2) = tmp;
     return val32;
 }
 
+/* This is to check if uint32 var will overflow if converted to signed long */
+#define PJ_CHECK_OVERFLOW_UINT32_TO_LONG(uint32_var, exec_on_overflow) \
+	do { \
+	    if (uint32_var > PJ_MAXLONG) { \
+		exec_on_overflow; \
+	    } \
+	} while (0)
 
 /**
  * @}
  */
 /**
diff --git a/pjlib/src/pj/file_io_ansi.c b/pjlib/src/pj/file_io_ansi.c
index d7a68cd649..0570b81c7a 100644
--- a/pjlib/src/pj/file_io_ansi.c
+++ b/pjlib/src/pj/file_io_ansi.c
@@ -18,10 +18,11 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA 
  */
 #include <pj/file_io.h>
 #include <pj/assert.h>
 #include <pj/errno.h>
+#include <pj/limits.h>
 #include <stdio.h>
 #include <errno.h>
 
 PJ_DEF(pj_status_t) pj_file_open( pj_pool_t *pool,
                                   const char *pathname, 
@@ -122,10 +123,16 @@ PJ_DEF(pj_status_t) pj_file_setpos( pj_oshandle_t fd,
                                     pj_off_t offset,
                                     enum pj_file_seek_type whence)
 {
     int mode;
 
+    if ((sizeof(pj_off_t) > sizeof(long)) &&
+        (offset > PJ_MAXLONG || offset < PJ_MINLONG)) 
+    {
+        return PJ_ENOTSUP;
+    }
+
     switch (whence) {
     case PJ_SEEK_SET:
         mode = SEEK_SET; break;
     case PJ_SEEK_CUR:
         mode = SEEK_CUR; break;
diff --git a/pjmedia/src/pjmedia/avi_player.c b/pjmedia/src/pjmedia/avi_player.c
index e22a148119..5690562e6d 100644
--- a/pjmedia/src/pjmedia/avi_player.c
+++ b/pjmedia/src/pjmedia/avi_player.c
@@ -280,11 +280,11 @@ pjmedia_avi_player_create_streams(pj_pool_t *pool,
     }
 
     /* Read the headers of each stream. */
     for (i = 0; i < avi_hdr.avih_hdr.num_streams; i++) {
         pj_size_t elem = 0;
-        pj_ssize_t size_to_read;
+        pj_off_t size_to_read;
 
         /* Read strl header */
         status = file_read(fport[0]->fd, &avi_hdr.strl_hdr[i],
                            sizeof(strl_hdr_t));
         if (status != PJ_SUCCESS)
@@ -333,10 +333,11 @@ pjmedia_avi_player_create_streams(pj_pool_t *pool,
 
     /* Skip any JUNK or LIST INFO until we get MOVI tag */
     do {
         pjmedia_avi_subchunk ch;
         int read = 0;
+        pj_off_t size_to_read;
 
         status = file_read(fport[0]->fd, &ch, sizeof(pjmedia_avi_subchunk));
         if (status != PJ_SUCCESS) {
             goto on_error;
         }
@@ -347,11 +348,19 @@ pjmedia_avi_player_create_streams(pj_pool_t *pool,
             status = file_read(fport[0]->fd, &ch, read);
             if (COMPARE_TAG(ch.id, PJMEDIA_AVI_MOVI_TAG))
                 break;
         }
 
-        status = pj_file_setpos(fport[0]->fd, ch.len-read, PJ_SEEK_CUR);
+        if (ch.len < read) {
+            status = PJ_EINVAL;
+            goto on_error;
+        }
+        PJ_CHECK_OVERFLOW_UINT32_TO_LONG(ch.len - read, 
+                                         status = PJ_EINVAL; goto on_error;);
+        size_to_read = (pj_off_t)ch.len - read;
+
+        status = pj_file_setpos(fport[0]->fd, size_to_read, PJ_SEEK_CUR);
         if (status != PJ_SUCCESS) {
             goto on_error;
         }
     } while(1);
 
@@ -773,10 +782,12 @@ static pj_status_t avi_get_frame(pjmedia_port *this_port,
         }
 
         /* Read new chunk data */
         if (fport->size_left == 0) {
             pj_off_t pos;
+            pj_off_t ch_len;
+
             pj_file_getpos(fport->fd, &pos);
 
             /* Data is padded to the nearest WORD boundary */
             if (fport->pad) {
                 status = pj_file_setpos(fport->fd, fport->pad, PJ_SEEK_CUR);
@@ -786,10 +797,14 @@ static pj_status_t avi_get_frame(pjmedia_port *this_port,
             status = file_read(fport->fd, &ch, sizeof(pjmedia_avi_subchunk));
             if (status != PJ_SUCCESS) {
                 size_read = 0;
                 goto on_error2;
             }
+            
+            PJ_CHECK_OVERFLOW_UINT32_TO_LONG(ch.len, 
+                                         status = PJ_EINVAL;  goto on_error2;);
+            ch_len = ch.len;
 
             cid = (char *)&ch.id;
             if (cid[0] >= '0' && cid[0] <= '9' &&
                 cid[1] >= '0' && cid[1] <= '9') 
             {
@@ -812,12 +827,11 @@ static pj_status_t avi_get_frame(pjmedia_port *this_port,
                            "AVIs in a single file."));
                     status = AVI_EOF;
                     goto on_error2;
                 }
 
-                status = pj_file_setpos(fport->fd, ch.len,
-                                        PJ_SEEK_CUR);
+                status = pj_file_setpos(fport->fd, ch_len, PJ_SEEK_CUR);
                 continue;
             }
             fport->size_left = ch.len;
         }
 
diff --git a/pjmedia/src/pjmedia/wav_player.c b/pjmedia/src/pjmedia/wav_player.c
index 2bd38bdbf3..8053bdc117 100644
--- a/pjmedia/src/pjmedia/wav_player.c
+++ b/pjmedia/src/pjmedia/wav_player.c
@@ -186,11 +186,12 @@ PJ_DEF(pj_status_t) pjmedia_wav_player_port_create( pj_pool_t *pool,
 						     unsigned options,
 						     pj_ssize_t buff_size,
 						     pjmedia_port **p_port )
 {
     pjmedia_wave_hdr wave_hdr;
-    pj_ssize_t size_to_read, size_read;
+    pj_ssize_t size_read;
+    pj_off_t size_to_read;
     struct file_reader_port *fport;
     pjmedia_audio_format_detail *ad;
     pj_off_t pos;
     pj_str_t name;
     unsigned samples_per_frame;
@@ -232,11 +233,11 @@ PJ_DEF(pj_status_t) pjmedia_wav_player_port_create( pj_pool_t *pool,
     status = pj_file_open( pool, filename, PJ_O_RDONLY, &fport->fd);
     if (status != PJ_SUCCESS)
 	return status;
 
     /* Read the file header plus fmt header only. */
-    size_read = size_to_read = sizeof(wave_hdr) - 8;
+    size_to_read = size_read = sizeof(wave_hdr) - 8;
     status = pj_file_read( fport->fd, &wave_hdr, &size_read);
     if (status != PJ_SUCCESS) {
 	pj_file_close(fport->fd);
 	return status;
     }
@@ -295,11 +296,13 @@ PJ_DEF(pj_status_t) pjmedia_wav_player_port_create( pj_pool_t *pool,
 
     /* If length of fmt_header is greater than 16, skip the remaining
      * fmt header data.
      */
     if (wave_hdr.fmt_hdr.len > 16) {
-	size_to_read = wave_hdr.fmt_hdr.len - 16;
+	PJ_CHECK_OVERFLOW_UINT32_TO_LONG(wave_hdr.fmt_hdr.len - 16,
+		      pj_file_close(fport->fd); return PJMEDIA_ENOTVALIDWAVE;);
+	size_to_read = (pj_off_t)wave_hdr.fmt_hdr.len - 16;
 	status = pj_file_setpos(fport->fd, size_to_read, PJ_SEEK_CUR);
 	if (status != PJ_SUCCESS) {
 	    pj_file_close(fport->fd);
 	    return status;
 	}
@@ -324,11 +327,14 @@ PJ_DEF(pj_status_t) pjmedia_wav_player_port_create( pj_pool_t *pool,
 	    wave_hdr.data_hdr.len = subchunk.len;
 	    break;
 	}
 
 	/* Otherwise skip the chunk contents */
+	PJ_CHECK_OVERFLOW_UINT32_TO_LONG(subchunk.len, 
+		      pj_file_close(fport->fd); return PJMEDIA_ENOTVALIDWAVE;);
 	size_to_read = subchunk.len;
+
 	status = pj_file_setpos(fport->fd, size_to_read, PJ_SEEK_CUR);
 	if (status != PJ_SUCCESS) {
 	    pj_file_close(fport->fd);
 	    return status;
 	}
diff --git a/pjmedia/src/pjmedia/wav_playlist.c b/pjmedia/src/pjmedia/wav_playlist.c
index b74eed39df..4e2c31e0c2 100644
--- a/pjmedia/src/pjmedia/wav_playlist.c
+++ b/pjmedia/src/pjmedia/wav_playlist.c
@@ -417,11 +417,12 @@ PJ_DEF(pj_status_t) pjmedia_wav_playlist_create(pj_pool_t *pool,
 
     /* ok run this for all files to be sure all are good for playback. */
     for (index=file_count-1; index>=0; index--) {
 
 	pjmedia_wave_hdr wavehdr;
-	pj_ssize_t size_to_read, size_read;
+	pj_ssize_t size_read;
+	pj_off_t size_to_read;
 
 	/* we end with the last one so we are good to go if still in function*/
 	pj_memcpy(filename, file_list[index].ptr, file_list[index].slen);
 	filename[file_list[index].slen] = '\0';
 
@@ -440,11 +441,11 @@ PJ_DEF(pj_status_t) pjmedia_wav_playlist_create(pj_pool_t *pool,
 			       &fport->fd_list[index]);
 	if (status != PJ_SUCCESS)
 	    goto on_error;
 	
 	/* Read the file header plus fmt header only. */
-	size_read = size_to_read = sizeof(wavehdr) - 8;
+	size_to_read = size_read = sizeof(wavehdr) - 8;	
 	status = pj_file_read( fport->fd_list[index], &wavehdr, &size_read);
 	if (status != PJ_SUCCESS) {
 	    goto on_error;
 	}
 
@@ -490,11 +491,13 @@ PJ_DEF(pj_status_t) pjmedia_wav_playlist_create(pj_pool_t *pool,
 	
 	/* If length of fmt_header is greater than 16, skip the remaining
 	 * fmt header data.
 	 */
 	if (wavehdr.fmt_hdr.len > 16) {
-	    size_to_read = wavehdr.fmt_hdr.len - 16;
+	    PJ_CHECK_OVERFLOW_UINT32_TO_LONG(wavehdr.fmt_hdr.len-16,
+			       status = PJMEDIA_ENOTVALIDWAVE; goto on_error;);
+	    size_to_read = (pj_off_t)wavehdr.fmt_hdr.len - 16;
 	    status = pj_file_setpos(fport->fd_list[index], size_to_read, 
 				    PJ_SEEK_CUR);
 	    if (status != PJ_SUCCESS) {
 		goto on_error;
 	    }
@@ -520,11 +523,14 @@ PJ_DEF(pj_status_t) pjmedia_wav_playlist_create(pj_pool_t *pool,
 		wavehdr.data_hdr.len = subchunk.len;
 		break;
 	    }
 	    
 	    /* Otherwise skip the chunk contents */
+	    PJ_CHECK_OVERFLOW_UINT32_TO_LONG(subchunk.len, 
+			       status = PJMEDIA_ENOTVALIDWAVE; goto on_error;);
 	    size_to_read = subchunk.len;
+
 	    status = pj_file_setpos(fport->fd_list[index], size_to_read, 
 				    PJ_SEEK_CUR);
 	    if (status != PJ_SUCCESS) {
 		goto on_error;
 	    }

Vulnerable code:         pj_ssize_t size_to_read;
        status = pj_file_setpos(fport[0]->fd, ch.len-read, PJ_SEEK_CUR);
                status = pj_file_setpos(fport->fd, ch.len,
                                        PJ_SEEK_CUR);
    pj_ssize_t size_to_read, size_read;
    size_read = size_to_read = sizeof(wave_hdr) - 8;
	size_to_read = wave_hdr.fmt_hdr.len - 16;
	pj_ssize_t size_to_read, size_read;
	size_read = size_to_read = sizeof(wavehdr) - 8;
	    size_to_read = wavehdr.fmt_hdr.len - 16;
Fixed code: #include <pj/limits.h>
/* This is to check if uint32 var will overflow if converted to signed long */
#define PJ_CHECK_OVERFLOW_UINT32_TO_LONG(uint32_var, exec_on_overflow) \
	do { \
	    if (uint32_var > PJ_MAXLONG) { \
		exec_on_overflow; \
	    } \
	} while (0)
#include <pj/limits.h>
    if ((sizeof(pj_off_t) > sizeof(long)) &&
        (offset > PJ_MAXLONG || offset < PJ_MINLONG)) 
    {
        return PJ_ENOTSUP;
    }

        pj_off_t size_to_read;
        pj_off_t size_to_read;
        if (ch.len < read) {
            status = PJ_EINVAL;
            goto on_error;
        }
        PJ_CHECK_OVERFLOW_UINT32_TO_LONG(ch.len - read, 
                                         status = PJ_EINVAL; goto on_error;);
        size_to_read = (pj_off_t)ch.len - read;

        status = pj_file_setpos(fport[0]->fd, size_to_read, PJ_SEEK_CUR);
            pj_off_t ch_len;

            
            PJ_CHECK_OVERFLOW_UINT32_TO_LONG(ch.len, 
                                         status = PJ_EINVAL;  goto on_error2;);
            ch_len = ch.len;
                status = pj_file_setpos(fport->fd, ch_len, PJ_SEEK_CUR);
    pj_ssize_t size_read;
    pj_off_t size_to_read;
    size_to_read = size_read = sizeof(wave_hdr) - 8;
	PJ_CHECK_OVERFLOW_UINT32_TO_LONG(wave_hdr.fmt_hdr.len - 16,
		      pj_file_close(fport->fd); return PJMEDIA_ENOTVALIDWAVE;);
	size_to_read = (pj_off_t)wave_hdr.fmt_hdr.len - 16;
	PJ_CHECK_OVERFLOW_UINT32_TO_LONG(subchunk.len, 
		      pj_file_close(fport->fd); return PJMEDIA_ENOTVALIDWAVE;);

	pj_ssize_t size_read;
	pj_off_t size_to_read;
	size_to_read = size_read = sizeof(wavehdr) - 8;	
	    PJ_CHECK_OVERFLOW_UINT32_TO_LONG(wavehdr.fmt_hdr.len-16,
			       status = PJMEDIA_ENOTVALIDWAVE; goto on_error;);
	    size_to_read = (pj_off_t)wavehdr.fmt_hdr.len - 16;
	    PJ_CHECK_OVERFLOW_UINT32_TO_LONG(subchunk.len, 
			       status = PJMEDIA_ENOTVALIDWAVE; goto on_error;);