From d5546ae5dbd24367a2d907cd2c545ab3bb15c3dd Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Wed, 15 May 2019 15:39:53 +0200 Subject: [PATCH 1/8] Fix incorrect format arguments in debug output printing Not caught by clang analyzer. --- CHANGELOG.md | 3 +++ src/dhcpoptinj.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fedb047..abe4ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Fixed +- Fix two format arguments in debug output printing (fairly pedantic; not even + caught by clang analyzer). ## 0.5.2 - 2019-05-09 ### Added diff --git a/src/dhcpoptinj.c b/src/dhcpoptinj.c index b4fdb89..b8c9068 100644 --- a/src/dhcpoptinj.c +++ b/src/dhcpoptinj.c @@ -586,7 +586,7 @@ static void debugLogOptions(void) if (!config->debug) return; - logMessage(LOG_DEBUG, "%u DHCP option(s) to inject (with a total of %zu bytes): ", + logMessage(LOG_DEBUG, "%zu DHCP option(s) to inject (with a total of %zu bytes): ", config->dhcpOptCodeCount, config->dhcpOptsSize); for (size_t i = 0; i < config->dhcpOptCodeCount; ++i) @@ -673,7 +673,7 @@ static void debugLogOption(const char *action, const struct DHCPOption *option) option->code, option->code, optName, - optNameLen > alignedWidth ? 0 : alignedWidth - optNameLen, + (int)(optNameLen > alignedWidth ? 0 : alignedWidth - optNameLen), "", option->length, optPayload); From 25dd4a32fe266a999ffe45a5abceafaf10e6ad15 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 30 May 2019 13:30:33 +0200 Subject: [PATCH 2/8] Limit DHCP option to 255 (not 256). Exit if option too long --- CHANGELOG.md | 2 ++ src/config.c | 13 ++++++++++--- src/dhcpoptinj.c | 4 ++-- src/options.c | 2 +- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abe4ae5..26bde81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix two format arguments in debug output printing (fairly pedantic; not even caught by clang analyzer). +- Limit DHCP options to 255 bytes (not 256). +- Exit if a DHCP option is too long. ## 0.5.2 - 2019-05-09 ### Added diff --git a/src/config.c b/src/config.c index f307c78..8ce5ad4 100644 --- a/src/config.c +++ b/src/config.c @@ -317,7 +317,7 @@ static void printHelp(void) "The option hex string is written as a series of two-digit pairs,\n" "optionally delimited by one or more non-hexadecimal characters:\n" "'466A6173','46 6A 61 73', '46:6A:61:73' etc. There is a maximum limit\n" - "of 256 bytes per option, excluding the option code (the first byte)\n" + "of 255 bytes per option, excluding the option code (the first byte)\n" "and the automatically inserted length byte. At least one option must\n" "be provided.\n" "\n" @@ -371,8 +371,8 @@ static void addDHCPOption(struct DHCPOptList *list, const char *string) if (!string) return; - /* Make room for length byte and payload */ - uint8_t buffer[1 + 256]; + /* Make room for option code byte and payload */ + uint8_t buffer[1 + UINT8_MAX]; size_t length = 0; for (size_t i = 0; i < strlen(string) && length < sizeof(buffer);) { @@ -384,8 +384,15 @@ static void addDHCPOption(struct DHCPOptList *list, const char *string) else ++i; } + /* Will not happen; the cmd.line parsing code expects an argument: */ if (!length) return; + if (length > UINT8_MAX) + { + fprintf(stderr, "DHCP option size exceeds the limit of %u bytes\n", + UINT8_MAX); + exit(EXIT_FAILURE); + } uint16_t optCode = buffer[0]; diff --git a/src/dhcpoptinj.c b/src/dhcpoptinj.c index b8c9068..e9886ed 100644 --- a/src/dhcpoptinj.c +++ b/src/dhcpoptinj.c @@ -654,9 +654,9 @@ static void debugLogOptionFound(const struct DHCPOption *option) static void debugLogOption(const char *action, const struct DHCPOption *option) { - /* String buffer for hex string (maximum DHCP option length (256) times + /* String buffer for hex string (maximum DHCP option length (255) times * three characters (two digits and a space)) */ - char optPayload[256 * 3]; + char optPayload[UINT8_MAX * 3]; size_t i = 0; for (; i < option->length; ++i) sprintf(optPayload + 3*i, "%02X ", option->data[i]); diff --git a/src/options.c b/src/options.c index c23c887..846db5b 100644 --- a/src/options.c +++ b/src/options.c @@ -28,7 +28,7 @@ struct DHCPOpt { uint8_t code; uint8_t length; - uint8_t data[256]; + uint8_t data[UINT8_MAX]; }; struct DHCPOptList From 43dcb09a2bc10d83a0fe86d09b263679407bdd82 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 30 May 2019 13:30:58 +0200 Subject: [PATCH 3/8] Bump version to 0.5.3 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4770252..e8730e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,7 +18,7 @@ endif() project( ${PROJECT} - VERSION 0.5.2 + VERSION 0.5.3 DESCRIPTION "DHCP option injector" LANGUAGES C ) From 4cef267562c865428a188997f2eed69576bc1de6 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 30 May 2019 14:47:48 +0200 Subject: [PATCH 4/8] Add .travis.yml, attempting automatic build --- .travis.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..387ddcb --- /dev/null +++ b/.travis.yml @@ -0,0 +1,15 @@ +addons: + apt: + update: true + packages: + - libnetfilter-queue-dev +language: c +compiler: + - clang + - gcc +before_script: + - mkdir build +script: + - cd build + - cmake .. + - make -j2 From d0b49cd3328c00dbb4dedcf251b2b21f05afdd1b Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 30 May 2019 15:00:02 +0200 Subject: [PATCH 5/8] Build on a slightly less super-ancient distribution (xenial) --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 387ddcb..0f95713 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ addons: update: true packages: - libnetfilter-queue-dev +dist: xenial language: c compiler: - clang From 6fee7e502236679353575f65e70cd24683603e04 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 30 May 2019 15:41:53 +0200 Subject: [PATCH 6/8] Suppress two clang-tidy warnings - config.c:532: the destination has just been allocated using the size of the source - dhcpoptinj.c:503: I cannot see the problem here; args1 is always initialised --- src/config.c | 1 + src/dhcpoptinj.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/config.c b/src/config.c index 8ce5ad4..539de8d 100644 --- a/src/config.c +++ b/src/config.c @@ -529,6 +529,7 @@ static void parseOption(struct Config *config, int option, char *arg, enum Sourc fputs("Could not allocate space for PID file name\n", stderr); exit(EXIT_FAILURE); } + // NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.strcpy) strcpy(config->pidFile, src); } break; diff --git a/src/dhcpoptinj.c b/src/dhcpoptinj.c index e9886ed..8c4c0fc 100644 --- a/src/dhcpoptinj.c +++ b/src/dhcpoptinj.c @@ -500,6 +500,7 @@ static void logMessage(int priority, const char *format, ...) if (priority == LOG_NOTICE || priority == LOG_INFO || priority == LOG_DEBUG) f = stdout; + /* NOLINTNEXTLINE(clang-analyzer-valist.Uninitialized) */ vfprintf(f, format, args1); } va_end(args1); From 42d6a29c45b364a5eec21298aa5d24df816909f6 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Thu, 1 Aug 2019 10:53:45 +0200 Subject: [PATCH 7/8] Add "badges" to README - Build status - Static analyser status (on master, using LGTM) - Package versions (repology) --- README.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 44bbded..d1501ed 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ # DHCP option injector +[![Build Status](https://travis-ci.org/misje/dhcpoptinj.svg?branch=dev)](https://travis-ci.org/misje/dhcpoptinj) [![Total alerts](https://img.shields.io/lgtm/alerts/g/misje/dhcpoptinj.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/misje/dhcpoptinj/alerts/) Have you ever wanted to intercept DHCP requests and squeeze in a few extra DHCP options, unbeknownst to the sender? Probably not. However, should the need ever @@ -104,13 +105,13 @@ Sending mangled packet ``` ## Installing +[![Packaging status](https://repology.org/badge/vertical-allrepos/dhcpoptinj.svg)](https://repology.org/project/dhcpoptinj/versions) -dhcpoptinj is submitted to Debian and will hopefully make it to unstable (and -consequently testing and stable) in not too long. The deb package is under -source control at [salsa](https://salsa.debian.org/misje-guest/dhcpoptinj). -Installing dhcpoptinj from the deb package is recommended over the following -manual installation procedure, because it also includes a man page, bash -completion rules, example files etc. +dhcpoptinj is in Debian/Ubuntu. The deb package is under source control at +[salsa](https://salsa.debian.org/misje-guest/dhcpoptinj). Installing +dhcpoptinj from the deb package is recommended over the following manual +installation procedure, because it also includes a man page, bash completion +rules, example files etc. ### Prerequisites From 32b9fb1c161a54806f19a1d50c980cca9d269b24 Mon Sep 17 00:00:00 2001 From: Andreas Misje Date: Tue, 6 Aug 2019 16:15:29 +0200 Subject: [PATCH 8/8] Update CHANGELOG, prepare for 0.5.3 release --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26bde81..562f36f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased + +## 0.5.3 - 2019-08-06 ### Fixed - Fix two format arguments in debug output printing (fairly pedantic; not even - caught by clang analyzer). + caught by clang analyser). - Limit DHCP options to 255 bytes (not 256). - Exit if a DHCP option is too long.