Skip to content

Avoid string duplications in ZEND_INI_MH(OnUpdateBaseDir) #20780

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

Draft
ndossche wants to merge 2 commits into
base: master
Choose a base branch
from
+10 −13
Draft

Avoid string duplications in ZEND_INI_MH(OnUpdateBaseDir) #20780

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a21d567
Avoid output string duplication in OnUpdateBaseDir MH handler
ndossche Dec 25, 2025
75e3b81
Avoid input string duplication in OnUpdateBaseDir MH handler
ndossche Dec 26, 2025
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
23 changes: 10 additions & 13 deletions main/fopen_wrappers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "zend_compile.h"
#include "php_network.h"
#include "zend_smart_str.h"
#include "zend_smart_string.h"

#ifdef HAVE_PWD_H
#include <pwd.h>
Expand Down Expand Up @@ -92,8 +93,8 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
}

/* Is the proposed open_basedir at least as restrictive as the current setting? */
smart_str buf = {0};
ptr = pathbuf = estrdup(ZSTR_VAL(new_value));
smart_string buf = {0};
ptr = pathbuf = ZSTR_VAL(new_value);
while (ptr && *ptr) {
end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
if (end != NULL) {
Expand All @@ -102,33 +103,29 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
}
char resolved_name[MAXPATHLEN + 1];
if (expand_filepath(ptr, resolved_name) == NULL) {
efree(pathbuf);
smart_str_free(&buf);
smart_string_free(&buf);
return FAILURE;
}
if (php_check_open_basedir_ex(resolved_name, 0) != 0) {
/* At least one portion of this open_basedir is less restrictive than the prior one, FAIL */
efree(pathbuf);
smart_str_free(&buf);
smart_string_free(&buf);
return FAILURE;
}
if (smart_str_get_len(&buf) != 0) {
smart_str_appendc(&buf, DEFAULT_DIR_SEPARATOR);
if (buf.len != 0) {
smart_string_appendc(&buf, DEFAULT_DIR_SEPARATOR);
}
smart_str_appends(&buf, resolved_name);
smart_string_appends(&buf, resolved_name);
ptr = end;
}
efree(pathbuf);

/* Everything checks out, set it */
zend_string *tmp = smart_str_extract(&buf);
char *result = estrdup(ZSTR_VAL(tmp));
smart_string_0(&buf);
char *result = buf.c;
Copy link
Member

@Girgias Girgias Dec 26, 2025

Choose a reason for hiding this comment

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

Shouldn't we maybe store the global as a zend_string? Surely open_dir checks require to know the length of the value? In which case we would need to change back to the smart_str API.

if (PG(open_basedir_modified)) {
efree(*p);
}
*p = result;
PG(open_basedir_modified) = true;
zend_string_release(tmp);

return SUCCESS;
}
Expand Down
Loading