halyssonJr commented on code in PR #3642:
URL: https://github.com/apache/nuttx-apps/pull/3642#discussion_r3637977208
##########
system/nxpkg/pkg_store.c:
##########
@@ -329,70 +368,167 @@ int pkg_store_read_text(FAR const char *path, FAR char
**buffer)
*buffer = NULL;
- stream = fopen(path, "rb");
- if (stream == NULL)
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
{
return errno == ENOENT ? -ENOENT : -errno;
}
- if (fseek(stream, 0, SEEK_END) < 0)
+ if (fstat(fd, &st) < 0)
{
- fclose(stream);
+ close(fd);
return -errno;
}
- length = ftell(stream);
- if (length < 0)
+ if (!S_ISREG(st.st_mode))
{
- fclose(stream);
- return -errno;
+ close(fd);
+ return -EINVAL;
}
- if (fseek(stream, 0, SEEK_SET) < 0)
+ /* Reject anything unreasonably large before the size is trusted for an
+ * allocation: guards both against a malicious/oversized text file (this
+ * path is used for the network-fetched index.jsn) and against
+ * "length + 1" wrapping if st_size were ever attacker-influenced up to
+ * SIZE_MAX.
+ */
+
+ if (st.st_size < 0 || st.st_size > (off_t)PKG_TEXT_MAX_SIZE)
{
- fclose(stream);
- return -errno;
+ close(fd);
+ return -EFBIG;
}
- data = malloc((size_t)length + 1);
+ length = (size_t)st.st_size;
+ data = pkg_malloc((size_t)length + 1);
if (data == NULL)
{
- fclose(stream);
+ close(fd);
return -ENOMEM;
}
- nread = fread(data, 1, (size_t)length, stream);
- if (nread != (size_t)length)
+ total = 0;
+ while (total < length)
{
- int err = ferror(stream);
+ ssize_t ret;
+
+ ret = read(fd, data + total, length - total);
+ if (ret < 0)
+ {
+ if (errno == EINTR)
+ {
+ continue;
+ }
+
+ close(fd);
+ pkg_free(data);
+ return -errno;
+ }
+
+ if (ret == 0)
+ {
+ break;
+ }
- fclose(stream);
- free(data);
- return err ? -EIO : -EINVAL;
+ total += (size_t)ret;
}
- fclose(stream);
+ nread = total;
+ close(fd);
+
+ if (nread != length)
+ {
+ pkg_free(data);
+ return -EINVAL;
+ }
data[length] = '\0';
*buffer = data;
return 0;
}
+#ifndef CONFIG_PSEUDOFS_FILE
+/****************************************************************************
+ * Name: pkg_store_make_tmp_path
+ *
+ * Description:
+ * Derive a staging path for an atomic write/copy to "path", under a
+ * short-name-compatible extension instead of appending ".tmp" (which
+ * would produce a second '.' in the final path component and break on
+ * FAT filesystems without long file name support).
+ *
+ ****************************************************************************/
+
+static int pkg_store_make_tmp_path(FAR char *tmp, size_t size,
+ FAR const char *path)
+{
+ FAR char *dot;
+ FAR char *slash;
+ int ret;
+
+ ret = snprintf(tmp, size, "%s", path);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ if ((size_t)ret >= size)
+ {
+ return -ENAMETOOLONG;
+ }
+
+ slash = strrchr(tmp, '/');
+ dot = strrchr(slash != NULL ? slash : tmp, '.');
+ if (dot != NULL)
+ {
+ *dot = '\0';
+ }
+
+ if (strlcat(tmp, ".tm", size) >= size)
+ {
+ return -ENAMETOOLONG;
+ }
+
+ return 0;
+}
+#endif
+
int pkg_store_write_text_atomic(FAR const char *path, FAR const char *text)
{
- char tmp[PATH_MAX];
+#ifdef CONFIG_PSEUDOFS_FILE
int fd;
int ret;
- ret = snprintf(tmp, sizeof(tmp), "%s.tmp", path);
+ fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+ if (fd < 0)
+ {
+ return -errno;
+ }
+
+ ret = pkg_store_write_all(fd, text, strlen(text));
if (ret < 0)
{
+ close(fd);
+ unlink(path);
return ret;
}
- if ((size_t)ret >= sizeof(tmp))
+ if (close(fd) < 0)
Review Comment:
Please use `ret` to receive the function's return value, and then you can
check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
