From 76d741c99035cd86dccb30002afd56decd3fa863 Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Fri, 30 Aug 2019 07:50:43 +0200 Subject: [PATCH 1/9] cgi-io: require whitelisting upload locations Introduce further ACL checks to verify that the request-supplied upload location may be written to. This prevents overwriting things like /bin/busybox and allows to confine uploads to specific directories. To setup the required ACLs, the following ubus command may be used on the command line: ubus call session grant '{ "ubus_rpc_session": "d41d8cd98f00b204e9800998ecf8427e", "scope": "cgi-io", "objects": [ [ "/etc/certificates/*", "write" ], [ "/var/uploads/*", "write" ] ] }' Signed-off-by: Jo-Philipp Wich (cherry picked from commit 22be9a1c0173a232d651059d84145bb6f51d3f67) --- net/cgi-io/src/main.c | 63 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index 2bfec623b..a6ded065f 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -263,6 +263,64 @@ postdecode(char **fields, int n_fields) return (found >= n_fields); } +static char * +canonicalize_path(const char *path, size_t len) +{ + char *canonpath, *cp; + const char *p, *e; + + if (path == NULL || *path == '\0') + return NULL; + + canonpath = datadup(path, len); + + if (canonpath == NULL) + return NULL; + + /* normalize */ + for (cp = canonpath, p = path, e = path + len; p < e; ) { + if (*p != '/') + goto next; + + /* skip repeating / */ + if ((p + 1 < e) && (p[1] == '/')) { + p++; + continue; + } + + /* /./ or /../ */ + if ((p + 1 < e) && (p[1] == '.')) { + /* skip /./ */ + if ((p + 2 >= e) || (p[2] == '/')) { + p += 2; + continue; + } + + /* collapse /x/../ */ + if ((p + 2 < e) && (p[2] == '.') && ((p + 3 >= e) || (p[3] == '/'))) { + while ((cp > canonpath) && (*--cp != '/')) + ; + + p += 3; + continue; + } + } + +next: + *cp++ = *p++; + } + + /* remove trailing slash if not root / */ + if ((cp > canonpath + 1) && (cp[-1] == '/')) + cp--; + else if (cp == canonpath) + *cp++ = '/'; + + *cp = '\0'; + + return canonpath; +} + static int response(bool success, const char *message) { @@ -417,6 +475,9 @@ data_begin_cb(multipart_parser *p) if (!st.filename) return response(false, "File data without name"); + if (!session_access(st.sessionid, st.filename, "write")) + return response(false, "Access to path denied by ACL"); + st.tempfd = mkstemp(tmpname); if (st.tempfd < 0) @@ -438,7 +499,7 @@ data_cb(multipart_parser *p, const char *data, size_t len) break; case PART_FILENAME: - st.filename = datadup(data, len); + st.filename = canonicalize_path(data, len); break; case PART_FILEMODE: From 0fc83858fc280b61980cd859704a039596a0e8bb Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Fri, 13 Sep 2019 06:52:21 +0200 Subject: [PATCH 2/9] cgi-io: use different acl scopes for path and command permissions Use the `cgi-io` scope to check for permission to execute the requested command (`upload`, `backup`) and the `file` scope to check path permissions. The reasoning of this change is that `cgi-io` is usually used in conjunction with `rpcd-mod-file` to transfer large file data out of band and `rpcd-mod-file` already uses the `file` scope to manage file path access permissions. After this change, both `rpc-mod-file` and `cgi-io` can share the same path acl rules. Write access to a path can be granted by using an ubus call in the following form: ubus call session grant '{ "ubus_rpc_session": "...", "scope": "file", "objects": [ [ "/var/lib/uploads/*", "write" ] ] }' Signed-off-by: Jo-Philipp Wich (cherry picked from commit c8a86c8c8e1925eb7d84c52e702c4be5fc8ac76b) --- net/cgi-io/Makefile | 2 +- net/cgi-io/src/main.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index 3a4fc565f..4b2d664af 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=7 +PKG_RELEASE:=9 PKG_LICENSE:=GPL-2.0-or-later diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index a6ded065f..44a520576 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -89,7 +89,7 @@ session_access_cb(struct ubus_request *req, int type, struct blob_attr *msg) } static bool -session_access(const char *sid, const char *obj, const char *func) +session_access(const char *sid, const char *scope, const char *obj, const char *func) { uint32_t id; bool allow = false; @@ -103,7 +103,7 @@ session_access(const char *sid, const char *obj, const char *func) blob_buf_init(&req, 0); blobmsg_add_string(&req, "ubus_rpc_session", sid); - blobmsg_add_string(&req, "scope", "cgi-io"); + blobmsg_add_string(&req, "scope", scope); blobmsg_add_string(&req, "object", obj); blobmsg_add_string(&req, "function", func); @@ -475,7 +475,7 @@ data_begin_cb(multipart_parser *p) if (!st.filename) return response(false, "File data without name"); - if (!session_access(st.sessionid, st.filename, "write")) + if (!session_access(st.sessionid, "file", st.filename, "write")) return response(false, "Access to path denied by ACL"); st.tempfd = mkstemp(tmpname); @@ -530,7 +530,7 @@ data_end_cb(multipart_parser *p) { if (st.parttype == PART_SESSIONID) { - if (!session_access(st.sessionid, "upload", "write")) + if (!session_access(st.sessionid, "cgi-io", "upload", "write")) { errno = EPERM; return response(false, "Upload permission denied"); @@ -658,7 +658,7 @@ main_backup(int argc, char **argv) char hostname[64] = { 0 }; char *fields[] = { "sessionid", NULL }; - if (!postdecode(fields, 1) || !session_access(fields[1], "backup", "read")) + if (!postdecode(fields, 1) || !session_access(fields[1], "cgi-io", "backup", "read")) return failure(0, "Backup permission denied"); if (pipe(fds)) From 13075d4d519ecff96441b54e40ef6509eb5ad836 Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Fri, 13 Sep 2019 07:23:25 +0200 Subject: [PATCH 3/9] cgi-io: add download operation Add a new `cgi-download` applet which allows to retrieve the contents of regular files or block devices. In order to initiate a transfer, a POST request in x-www-form-urlencoded format must be sent to the applet, with one field "sessionid" holding the login session and another field "path" containing the file path to download. Further optional fields are "filename" which - if present - will cause the download applet to set a Content-Dispostition header and "mimetype" which allows to let the applet respond with a specific type instead of the default "application/octet-stream". Below is an example for the required acl rules to grant download access to files or block devices: ubus call session grant '{ "ubus_rpc_session": "...", "scope": "cgi-io", "objects": [ [ "download", "read" ] ] }' ubus call session grant '{ "ubus_rpc_session": "...", "scope": "file", "objects": [ [ "/etc/config/*", "read" ], [ "/dev/mtdblock*", "read" ] ] }' Signed-off-by: Jo-Philipp Wich (cherry picked from commit ab2a2b080d4143bfbbd8584a39999ef998905dd2) --- net/cgi-io/Makefile | 3 +- net/cgi-io/src/main.c | 83 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index 4b2d664af..5ba695fae 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=9 +PKG_RELEASE:=10 PKG_LICENSE:=GPL-2.0-or-later @@ -38,6 +38,7 @@ define Package/cgi-io/install $(INSTALL_DIR) $(1)/usr/libexec $(1)/www/cgi-bin/ $(INSTALL_BIN) $(PKG_BUILD_DIR)/cgi-io $(1)/usr/libexec $(LN) ../../usr/libexec/cgi-io $(1)/www/cgi-bin/cgi-upload + $(LN) ../../usr/libexec/cgi-io $(1)/www/cgi-bin/cgi-download $(LN) ../../usr/libexec/cgi-io $(1)/www/cgi-bin/cgi-backup endef diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index 44a520576..aaba37d0d 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -26,6 +26,9 @@ #include #include #include +#include +#include +#include #include #include @@ -645,6 +648,84 @@ main_upload(int argc, char *argv[]) return 0; } +static int +main_download(int argc, char **argv) +{ + char *fields[] = { "sessionid", NULL, "path", NULL, "filename", NULL, "mimetype", NULL }; + unsigned long long size = 0; + char *p, buf[4096]; + ssize_t len = 0; + struct stat s; + int rfd; + + postdecode(fields, 4); + + if (!fields[1] || !session_access(fields[1], "cgi-io", "download", "read")) + return failure(0, "Download permission denied"); + + if (!fields[3] || !session_access(fields[1], "file", fields[3], "read")) + return failure(0, "Access to path denied by ACL"); + + if (stat(fields[3], &s)) + return failure(errno, "Failed to stat requested path"); + + if (!S_ISREG(s.st_mode) && !S_ISBLK(s.st_mode)) + return failure(0, "Requested path is not a regular file or block device"); + + for (p = fields[5]; p && *p; p++) + if (!isalnum(*p) && !strchr(" ()<>@,;:[]?.=%", *p)) + return failure(0, "Invalid characters in filename"); + + for (p = fields[7]; p && *p; p++) + if (!isalnum(*p) && !strchr(" .;=/-", *p)) + return failure(0, "Invalid characters in mimetype"); + + rfd = open(fields[3], O_RDONLY); + + if (rfd < 0) + return failure(errno, "Failed to open requested path"); + + if (S_ISBLK(s.st_mode)) + ioctl(rfd, BLKGETSIZE64, &size); + else + size = (unsigned long long)s.st_size; + + printf("Status: 200 OK\r\n"); + printf("Content-Type: %s\r\n", fields[7] ? fields[7] : "application/octet-stream"); + + if (fields[5]) + printf("Content-Disposition: attachment; filename=\"%s\"\r\n", fields[5]); + + printf("Content-Length: %llu\r\n\r\n", size); + fflush(stdout); + + while (size > 0) { + len = sendfile(1, rfd, NULL, size); + + if (len == -1) { + if (errno == ENOSYS || errno == EINVAL) { + while ((len = read(rfd, buf, sizeof(buf))) > 0) + fwrite(buf, len, 1, stdout); + + fflush(stdout); + break; + } + + if (errno == EINTR || errno == EAGAIN) + continue; + } + + if (len <= 0) + break; + + size -= len; + } + + close(rfd); + + return 0; +} + static int main_backup(int argc, char **argv) { @@ -718,6 +799,8 @@ int main(int argc, char **argv) { if (strstr(argv[0], "cgi-upload")) return main_upload(argc, argv); + else if (strstr(argv[0], "cgi-download")) + return main_download(argc, argv); else if (strstr(argv[0], "cgi-backup")) return main_backup(argc, argv); From a1e87b4e0e49d157c027b080b6dd202257aeb5b1 Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Fri, 13 Sep 2019 08:32:58 +0200 Subject: [PATCH 4/9] cgi-io: pass appropriate HTTP error codes to failure() Instead of always replying with a generic 500 internal server error code, use more appropriate codes such as 403 to indicate denied permissions. Signed-off-by: Jo-Philipp Wich (cherry picked from commit 8c22db653158e8c4edf4fdd0e0554a603b96a655) --- net/cgi-io/Makefile | 2 +- net/cgi-io/src/main.c | 26 ++++++++++++++------------ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index 5ba695fae..eaf03b40d 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=10 +PKG_RELEASE:=11 PKG_LICENSE:=GPL-2.0-or-later diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index aaba37d0d..d19277d6a 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -369,15 +369,17 @@ response(bool success, const char *message) } static int -failure(int e, const char *message) +failure(int code, int e, const char *message) { - printf("Status: 500 Internal Server failure\r\n"); + printf("Status: %d %s\r\n", code, message); printf("Content-Type: text/plain\r\n\r\n"); printf("%s", message); if (e) printf(": %s", strerror(e)); + printf("\n"); + return -1; } @@ -661,29 +663,29 @@ main_download(int argc, char **argv) postdecode(fields, 4); if (!fields[1] || !session_access(fields[1], "cgi-io", "download", "read")) - return failure(0, "Download permission denied"); + return failure(403, 0, "Download permission denied"); if (!fields[3] || !session_access(fields[1], "file", fields[3], "read")) - return failure(0, "Access to path denied by ACL"); + return failure(403, 0, "Access to path denied by ACL"); if (stat(fields[3], &s)) - return failure(errno, "Failed to stat requested path"); + return failure(404, errno, "Failed to stat requested path"); if (!S_ISREG(s.st_mode) && !S_ISBLK(s.st_mode)) - return failure(0, "Requested path is not a regular file or block device"); + return failure(403, 0, "Requested path is not a regular file or block device"); for (p = fields[5]; p && *p; p++) if (!isalnum(*p) && !strchr(" ()<>@,;:[]?.=%", *p)) - return failure(0, "Invalid characters in filename"); + return failure(400, 0, "Invalid characters in filename"); for (p = fields[7]; p && *p; p++) if (!isalnum(*p) && !strchr(" .;=/-", *p)) - return failure(0, "Invalid characters in mimetype"); + return failure(400, 0, "Invalid characters in mimetype"); rfd = open(fields[3], O_RDONLY); if (rfd < 0) - return failure(errno, "Failed to open requested path"); + return failure(500, errno, "Failed to open requested path"); if (S_ISBLK(s.st_mode)) ioctl(rfd, BLKGETSIZE64, &size); @@ -740,15 +742,15 @@ main_backup(int argc, char **argv) char *fields[] = { "sessionid", NULL }; if (!postdecode(fields, 1) || !session_access(fields[1], "cgi-io", "backup", "read")) - return failure(0, "Backup permission denied"); + return failure(403, 0, "Backup permission denied"); if (pipe(fds)) - return failure(errno, "Failed to spawn pipe"); + return failure(500, errno, "Failed to spawn pipe"); switch ((pid = fork())) { case -1: - return failure(errno, "Failed to fork process"); + return failure(500, errno, "Failed to fork process"); case 0: dup2(fds[1], 1); From 92bea7f8e935a0a227696467c03c38209b270190 Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Fri, 13 Sep 2019 09:17:58 +0200 Subject: [PATCH 5/9] cgi-io: use splice() to stream backup archive This improves the I/O performance when outputting large backups. Signed-off-by: Jo-Philipp Wich (cherry picked from commit a8b4a28372645c93621008b2563c00ce6cdd739f) --- net/cgi-io/Makefile | 2 +- net/cgi-io/src/main.c | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index eaf03b40d..211360905 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=11 +PKG_RELEASE:=12 PKG_LICENSE:=GPL-2.0-or-later diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index d19277d6a..ca1575842 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -16,6 +16,8 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#define _GNU_SOURCE /* splice(), SPLICE_F_MORE */ + #include #include #include @@ -736,7 +738,6 @@ main_backup(int argc, char **argv) int len; int status; int fds[2]; - char buf[4096]; char datestr[16] = { 0 }; char hostname[64] = { 0 }; char *fields[] = { "sessionid", NULL }; @@ -768,7 +769,6 @@ main_backup(int argc, char **argv) return -1; default: - fcntl(fds[0], F_SETFL, fcntl(fds[0], F_GETFL) | O_NONBLOCK); now = time(NULL); strftime(datestr, sizeof(datestr) - 1, "%Y-%m-%d", localtime(&now)); @@ -780,15 +780,13 @@ main_backup(int argc, char **argv) printf("Content-Disposition: attachment; " "filename=\"backup-%s-%s.tar.gz\"\r\n\r\n", hostname, datestr); + fflush(stdout); + do { - waitpid(pid, &status, 0); + len = splice(fds[0], NULL, 1, NULL, 4096, SPLICE_F_MORE); + } while (len > 0); - while ((len = read(fds[0], buf, sizeof(buf))) > 0) { - fwrite(buf, len, 1, stdout); - fflush(stdout); - } - - } while (!WIFEXITED(status)); + waitpid(pid, &status, 0); close(fds[0]); close(fds[1]); From af5585ac88eb3dbcef8c6c7dbe662940ae1d558e Mon Sep 17 00:00:00 2001 From: Ansuel Smith Date: Tue, 8 Oct 2019 22:34:11 +0200 Subject: [PATCH 6/9] cgi-io: fix read after end errors Currently cgi-io try to read data after the data ended. - Adds "-" to whitelist char - In main_upload is tried to consume the buffer while it's already readed by the while loop before Signed-off-by: Ansuel Smith (cherry picked from commit 535b2b6bd8a7f7a0a7a6914c8091619ea6f8961f) --- net/cgi-io/Makefile | 2 +- net/cgi-io/src/main.c | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index 211360905..6bc906ec5 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=12 +PKG_RELEASE:=13 PKG_LICENSE:=GPL-2.0-or-later diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index ca1575842..e4d0b212f 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -37,6 +37,7 @@ #include "multipart_parser.h" +#define READ_BLOCK 4096 enum part { PART_UNKNOWN, @@ -389,7 +390,7 @@ static int filecopy(void) { int len; - char buf[4096]; + char buf[READ_BLOCK]; if (!st.filedata) { @@ -625,7 +626,8 @@ static int main_upload(int argc, char *argv[]) { int rem, len; - char buf[4096]; + bool done = false; + char buf[READ_BLOCK]; multipart_parser *p; p = init_parser(); @@ -638,17 +640,14 @@ main_upload(int argc, char *argv[]) while ((len = read(0, buf, sizeof(buf))) > 0) { - rem = multipart_parser_execute(p, buf, len); - - if (rem < len) - break; + if (!done) { + rem = multipart_parser_execute(p, buf, len); + done = (rem < len); + } } multipart_parser_free(p); - /* read remaining post data */ - while ((len = read(0, buf, sizeof(buf))) > 0); - return 0; } @@ -657,7 +656,7 @@ main_download(int argc, char **argv) { char *fields[] = { "sessionid", NULL, "path", NULL, "filename", NULL, "mimetype", NULL }; unsigned long long size = 0; - char *p, buf[4096]; + char *p, buf[READ_BLOCK]; ssize_t len = 0; struct stat s; int rfd; @@ -677,7 +676,7 @@ main_download(int argc, char **argv) return failure(403, 0, "Requested path is not a regular file or block device"); for (p = fields[5]; p && *p; p++) - if (!isalnum(*p) && !strchr(" ()<>@,;:[]?.=%", *p)) + if (!isalnum(*p) && !strchr(" ()<>@,;:[]?.=%-", *p)) return failure(400, 0, "Invalid characters in filename"); for (p = fields[7]; p && *p; p++) @@ -783,7 +782,7 @@ main_backup(int argc, char **argv) fflush(stdout); do { - len = splice(fds[0], NULL, 1, NULL, 4096, SPLICE_F_MORE); + len = splice(fds[0], NULL, 1, NULL, READ_BLOCK, SPLICE_F_MORE); } while (len > 0); waitpid(pid, &status, 0); From 0698c1ab29c25e56e8d394f3036dd3d4c6579947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Fri, 11 Oct 2019 15:01:42 +0200 Subject: [PATCH 7/9] cgi-io: cmake: fix libraries lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to make it compile properly in more environments. Signed-off-by: Petr Štetiar (cherry picked from commit fd47e99be4fcd9b1261a4f359279d63199fff6c3) --- net/cgi-io/src/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/cgi-io/src/CMakeLists.txt b/net/cgi-io/src/CMakeLists.txt index 6d8b1585a..56d9fa7f1 100644 --- a/net/cgi-io/src/CMakeLists.txt +++ b/net/cgi-io/src/CMakeLists.txt @@ -5,6 +5,8 @@ PROJECT(cgi-io C) INCLUDE(CheckFunctionExists) FIND_PATH(ubus_include_dir libubus.h) +FIND_LIBRARY(ubox NAMES ubox) +FIND_LIBRARY(ubus NAMES ubus) INCLUDE_DIRECTORIES(${ubus_include_dir}) ADD_DEFINITIONS(-Os -Wall -Werror --std=gnu99 -g3 -Wmissing-declarations) @@ -17,6 +19,6 @@ IF(APPLE) ENDIF() ADD_EXECUTABLE(cgi-io main.c multipart_parser.c) -TARGET_LINK_LIBRARIES(cgi-io ubox ubus) +TARGET_LINK_LIBRARIES(cgi-io ${ubox} ${ubus}) INSTALL(TARGETS cgi-io RUNTIME DESTINATION sbin) From 6677274844a13ae5cfa2a584c52396af0ab1682a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Fri, 11 Oct 2019 15:03:04 +0200 Subject: [PATCH 8/9] cgi-io: cmake: enable extra compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spotting issues during compilation is cheaper. Signed-off-by: Petr Štetiar (cherry picked from commit 4e7411a8d0a46363f9946bff762eda70d5d5de6c) --- net/cgi-io/src/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/cgi-io/src/CMakeLists.txt b/net/cgi-io/src/CMakeLists.txt index 56d9fa7f1..c7c9d40ca 100644 --- a/net/cgi-io/src/CMakeLists.txt +++ b/net/cgi-io/src/CMakeLists.txt @@ -9,7 +9,8 @@ FIND_LIBRARY(ubox NAMES ubox) FIND_LIBRARY(ubus NAMES ubus) INCLUDE_DIRECTORIES(${ubus_include_dir}) -ADD_DEFINITIONS(-Os -Wall -Werror --std=gnu99 -g3 -Wmissing-declarations) +ADD_DEFINITIONS(-Os -Wall -Werror -Wextra --std=gnu99 -g3) +ADD_DEFINITIONS(-Wno-unused-parameter -Wmissing-declarations) SET(CMAKE_SHARED_LIBRARY_LINK_C_FLAGS "") From 6a4c0cab441a86ae0664e5b2cfd20d61945b2c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0tetiar?= Date: Fri, 11 Oct 2019 15:07:17 +0200 Subject: [PATCH 9/9] cgi-io: iron out extra compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes following errors: main.c:458:37: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] main.c:463:17: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare] main.c:518:35: error: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] main.c:157:3: error: ignoring return value of ‘read’, declared with attribute warn_unused_result [-Werror=unused-result] main.c:763:3: error: ignoring return value of ‘chdir’, declared with attribute warn_unused_result [-Werror=unused-result] Signed-off-by: Petr Štetiar (cherry picked from commit bb6cdb804cc4db12cca776f559baa6d989a992ec) --- net/cgi-io/Makefile | 2 +- net/cgi-io/src/main.c | 20 +++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/net/cgi-io/Makefile b/net/cgi-io/Makefile index 6bc906ec5..92cca4fa0 100644 --- a/net/cgi-io/Makefile +++ b/net/cgi-io/Makefile @@ -8,7 +8,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=cgi-io -PKG_RELEASE:=13 +PKG_RELEASE:=14 PKG_LICENSE:=GPL-2.0-or-later diff --git a/net/cgi-io/src/main.c b/net/cgi-io/src/main.c index e4d0b212f..3530284c6 100644 --- a/net/cgi-io/src/main.c +++ b/net/cgi-io/src/main.c @@ -126,6 +126,7 @@ static char * checksum(const char *applet, size_t sumlen, const char *file) { pid_t pid; + int r; int fds[2]; static char chksum[65]; @@ -154,10 +155,14 @@ checksum(const char *applet, size_t sumlen, const char *file) default: memset(chksum, 0, sizeof(chksum)); - read(fds[0], chksum, sumlen); + r = read(fds[0], chksum, sumlen); + waitpid(pid, NULL, 0); close(fds[0]); close(fds[1]); + + if (r < 0) + return NULL; } return chksum; @@ -442,7 +447,7 @@ header_field(multipart_parser *p, const char *data, size_t len) static int header_value(multipart_parser *p, const char *data, size_t len) { - int i, j; + size_t i, j; if (!st.is_content_disposition) return 0; @@ -500,6 +505,8 @@ data_begin_cb(multipart_parser *p) static int data_cb(multipart_parser *p, const char *data, size_t len) { + int wlen = len; + switch (st.parttype) { case PART_SESSIONID: @@ -515,14 +522,14 @@ data_cb(multipart_parser *p, const char *data, size_t len) break; case PART_FILEDATA: - if (write(st.tempfd, data, len) != len) + if (write(st.tempfd, data, len) != wlen) { close(st.tempfd); return response(false, "I/O failure while writing temporary file"); } if (!st.filedata) - st.filedata = !!len; + st.filedata = !!wlen; break; @@ -734,6 +741,7 @@ main_backup(int argc, char **argv) { pid_t pid; time_t now; + int r; int len; int status; int fds[2]; @@ -760,7 +768,9 @@ main_backup(int argc, char **argv) close(fds[0]); close(fds[1]); - chdir("/"); + r = chdir("/"); + if (r < 0) + return failure(500, errno, "Failed chdir('/')"); execl("/sbin/sysupgrade", "/sbin/sysupgrade", "--create-backup", "-", NULL);