]> git.ipfire.org Git - thirdparty/FORT-validator.git/commitdiff
File: Patch undefined behavior
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Thu, 3 Feb 2022 21:43:27 +0000 (15:43 -0600)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Thu, 3 Feb 2022 21:43:50 +0000 (15:43 -0600)
Code was falling into the `strchr()` trap:

last_slash = strrchr(path, '/');
*last_slash = '\0';

`path` is const, and `strrchr()` returns a pointer to the same string,
so the second line was risking a const edit.

Also fixes some leftover compilation problems.

src/file.c
src/http/http.c
src/rrdp/delta.c
src/rrdp/snapshot.c
src/rrdp/types.c
src/rrdp/types.h

index cdf7e80a9a14e9a933229ebcf206e6dba7aa1ead..8cfbfe4a833fb9ca5e07c08c8d2aee29ccdab46c 100644 (file)
@@ -300,13 +300,24 @@ valid_file_or_dir(char const *location, bool check_file, bool check_dir,
 }
 
 static int
-dir_exists(char const *path, bool *result)
+dir_exists(char const *_path, bool *result)
 {
-       struct stat _stat;
+       char *path;
        char *last_slash;
+       struct stat _stat;
+       int error;
+
+       /*
+        * Need to remove the const, otherwise "= '\0'" below causes undefined
+        * behavior.
+        */
+       path = strdup(_path);
+       if (path == NULL)
+               return pr_enomem();
 
        last_slash = strrchr(path, '/');
        if (last_slash == NULL) {
+               free(path);
                /*
                 * Simply because create_dir_recursive() has nothing meaningful
                 * to do when this happens. It's a pretty strange error.
@@ -319,17 +330,21 @@ dir_exists(char const *path, bool *result)
 
        if (stat(path, &_stat) == 0) {
                if (!S_ISDIR(_stat.st_mode)) {
-                       return pr_op_err("Path '%s' exists and is not a directory.",
+                       error = pr_op_err("Path '%s' exists and is not a directory.",
                            path);
+                       free(path);
+                       return error;
                }
                *result = true;
        } else if (errno == ENOENT) {
                *result = false;
        } else {
-               return pr_op_errno(errno, "stat() failed");
+               error = errno;
+               free(path);
+               return pr_op_errno(error, "stat() failed");
        }
 
-       *last_slash = '/';
+       free(path);
        return 0;
 }
 
index ad4e6c6475a5e41188a9d9e2a58891b0cae84a33..f8438518ec7b0511f65e1e4ab48100a05d754b0a 100644 (file)
@@ -352,7 +352,7 @@ retry_until_done(char const *remote, char const *local, long ims)
  * Return values:
  *
  * - 0: Download successful.
- * - ENOTCHANGED: File hasn't changed since `args->ims`.
+ * - ENOTCHANGED: File hasn't changed since @ims.
  * - < 0: Something went wrong.
  */
 int
index 27c78542ebc479f274f71714a85b3308b7513c45..053dafa4df1023a0e3ecb1d6ae6318cf32f7394c 100644 (file)
@@ -71,7 +71,7 @@ xml_read_delta(xmlTextReaderPtr reader, void *arg)
        name = xmlTextReaderConstLocalName(reader);
 
        if (xmlStrEqual(name, PUBLISH))
-               return handle_publish_tag(reader, true, ctx->notification);
+               return handle_publish_tag(reader, ctx->notification, true);
        if (xmlStrEqual(name, WITHDRAW))
                return handle_withdraw_tag(reader, ctx->notification);
        if (xmlStrEqual(name, DELTA)) {
index 5ee1290cd45061120065601c4af09eeb5d4d9cc7..9adeb2c609a4896d58889b8102535ca2fe6504cf 100644 (file)
@@ -1,6 +1,7 @@
 #include "rrdp/snapshot.h"
 
 #define _XOPEN_SOURCE 500
+#define __USE_XOPEN_EXTENDED 1
 #include <ftw.h>
 
 #include "thread_var.h"
@@ -29,7 +30,7 @@ clear_caged_directory(struct rrdp_notification *notif)
        struct rpki_uri *cage;
        int error;
 
-       error = uri_create_caged(NULL, notif, &cage);
+       error = uri_create_caged(NULL, notif->uri, &cage);
        if (error)
                return error;
 
index e0f208d716538107029889e717b2295c790fb047..c40076391053f7a66f268f39680b41886c797986 100644 (file)
@@ -422,14 +422,9 @@ write_from_uri(struct rrdp_publish *publish)
 
        file = uri_get_local(publish->target.uri);
 
-       /*
-        * TODO (aaaa) shouldn't this be reverted on error?
-        * Also, shouldn't create_dir_recursive() be inside file_write()?
-        */
        error = create_dir_recursive(file);
        if (error)
                return error;
-
        error = file_write(file, &out);
        if (error)
                return error;
@@ -439,6 +434,7 @@ write_from_uri(struct rrdp_publish *publish)
 
        file_close(out);
 
+       /* fwrite does not yield an error message... */
        if (written != (sizeof(unsigned char) * publish->content_len))
                return pr_val_err("Couldn't write bytes to file '%s'", file);
 
index b4748084dfbf67fcd237995485cf8f84a8436a54..46cb3cedb099c979c535f9d6baa341c64fb8a00e 100644 (file)
@@ -54,6 +54,6 @@ int parse_hash_attribute(xmlTextReaderPtr, bool, struct rrdp_file_metadata *);
 
 int parse_header_tag(xmlTextReaderPtr, struct rrdp_session *);
 int validate_header_tag(xmlTextReaderPtr, struct rrdp_session *);
-int handle_publish_tag(xmlTextReaderPtr, struct rrdp_notification *);
+int handle_publish_tag(xmlTextReaderPtr, struct rrdp_notification *, bool);
 
 #endif /* SRC_RRDP_TYPES_H_ */