Skip to content

Avoid using unsafe sprintf() #19598

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+17 −51
Open

Avoid using unsafe sprintf() #19598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions Zend/zend_alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,24 +379,6 @@ static const uint32_t bin_pages[] = {
ZEND_MM_BINS_INFO(_BIN_DATA_PAGES, x, y)
};

#if ZEND_DEBUG
ZEND_COLD void zend_debug_alloc_output(char *format, ...)
{
char output_buf[256];
va_list args;

va_start(args, format);
vsprintf(output_buf, format, args);
va_end(args);

#ifdef ZEND_WIN32
OutputDebugString(output_buf);
#else
fprintf(stderr, "%s", output_buf);
#endif
}
#endif

static ZEND_COLD ZEND_NORETURN void zend_mm_panic(const char *message)
{
fprintf(stderr, "%s\n", message);
Expand Down
10 changes: 4 additions & 6 deletions ext/pdo_firebird/firebird_statement.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ static int get_formatted_time_tz(pdo_stmt_t *stmt, const ISC_TIME_TZ* timeTz, zv
struct tm t;
ISC_TIME time;
char timeBuf[80] = {0};
char timeTzBuf[124] = {0};
if (fb_decode_time_tz(S->H->isc_status, timeTz, &hours, &minutes, &seconds, &fractions, sizeof(timeZoneBuffer), timeZoneBuffer)) {
return 1;
}
Expand All @@ -100,8 +99,8 @@ static int get_formatted_time_tz(pdo_stmt_t *stmt, const ISC_TIME_TZ* timeTz, zv
return 1;
}

size_t time_tz_len = sprintf(timeTzBuf, "%s %s", timeBuf, timeZoneBuffer);
ZVAL_STRINGL(result, timeTzBuf, time_tz_len);
zend_string *time_tz_str = zend_strpprintf(0, "%s %s", timeBuf, timeZoneBuffer);
ZVAL_NEW_STR(result, time_tz_str);
return 0;
}

Expand All @@ -115,7 +114,6 @@ static int get_formatted_timestamp_tz(pdo_stmt_t *stmt, const ISC_TIMESTAMP_TZ*
struct tm t;
ISC_TIMESTAMP ts;
char timestampBuf[80] = {0};
char timestampTzBuf[124] = {0};
if (fb_decode_timestamp_tz(S->H->isc_status, timestampTz, &year, &month, &day, &hours, &minutes, &seconds, &fractions, sizeof(timeZoneBuffer), timeZoneBuffer)) {
return 1;
}
Expand All @@ -130,8 +128,8 @@ static int get_formatted_timestamp_tz(pdo_stmt_t *stmt, const ISC_TIMESTAMP_TZ*
return 1;
}

size_t timestamp_tz_len = sprintf(timestampTzBuf, "%s %s", timestampBuf, timeZoneBuffer);
ZVAL_STRINGL(result, timestampTzBuf, timestamp_tz_len);
zend_string *timestamp_tz_str = zend_strpprintf(0, "%s %s", timestampBuf, timeZoneBuffer);
ZVAL_NEW_STR(result, timestamp_tz_str);
return 0;
}

Expand Down
40 changes: 13 additions & 27 deletions sapi/fpm/fpm/fpm_sockets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "fpm_env.h"
#include "fpm_cleanup.h"
#include "fpm_scoreboard.h"
#include "zend_smart_string.h"

struct listening_socket_s {
int refcount;
Expand Down Expand Up @@ -58,40 +59,24 @@ static void fpm_sockets_cleanup(int which, void *arg) /* {{{ */
unsigned i;
unsigned socket_set_count = 0;
unsigned socket_set[FPM_ENV_SOCKET_SET_MAX];
unsigned socket_set_buf = 0;
char envname[32];
char *env_value = 0;
int p = 0;
smart_string env_str = {0};
struct listening_socket_s *ls = sockets_list.data;

for (i = 0; i < sockets_list.used; i++, ls++) {
if (which != FPM_CLEANUP_PARENT_EXEC) {
close(ls->sock);
} else { /* on PARENT EXEC we want socket fds to be inherited through environment variable */
char fd[32];
char *tmpenv_value;
snprintf(fd, sizeof(fd), "%d", ls->sock);

socket_set_buf = (i % FPM_ENV_SOCKET_SET_SIZE == 0 && i) ? 1 : 0;
tmpenv_value = realloc(env_value, p + (p ? 1 : 0) + strlen(ls->key) + 1 + strlen(fd) + socket_set_buf + 1);
if (!tmpenv_value) {
zlog(ZLOG_SYSERROR, "failure to inherit data on parent exec for socket `%s` due to memory allocation failure", ls->key);
free(ls->key);
break;
}

env_value = tmpenv_value;

if (i % FPM_ENV_SOCKET_SET_SIZE == 0) {
socket_set[socket_set_count] = p + socket_set_buf;
smart_string_appendc(&env_str, '\0');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is master process code - it should not use zend alloc. Ideally all master code should also not immediately exit during the alloc failure so it should keep using standard malloc / realloc / free. It means even the persistent version would not be preferred here. I would prefer not the use smart string at all here

socket_set[socket_set_count] = env_str.len;
socket_set_count++;
if (i) {
*(env_value + p + 1) = 0;
}
} else {
smart_string_appendc(&env_str, ',');
}

p += sprintf(env_value + p + socket_set_buf, "%s%s=%s", (p && !socket_set_buf) ? "," : "", ls->key, fd);
p += socket_set_buf;
smart_string_appends(&env_str, ls->key);
smart_string_appendc(&env_str, '=');
smart_string_append_long(&env_str, ls->sock);
}

if (which == FPM_CLEANUP_PARENT_EXIT_MAIN) {
Expand All @@ -102,14 +87,15 @@ static void fpm_sockets_cleanup(int which, void *arg) /* {{{ */
free(ls->key);
}

if (env_value) {
if (env_str.c) {
smart_string_appendc(&env_str, '\0');
for (i = 0; i < socket_set_count; i++) {
fpm_sockets_get_env_name(envname, sizeof(envname), i);
setenv(envname, env_value + socket_set[i], 1);
setenv(envname, env_str.c + socket_set[i], 1);
}
fpm_sockets_get_env_name(envname, sizeof(envname), socket_set_count);
unsetenv(envname);
free(env_value);
smart_string_free(&env_str);
}

fpm_array_free(&sockets_list);
Expand Down