Fix for `zstd` CLI accepts bogus values for numeric parameters (#3268)

* add checks to mal-formed numeric values for memory and memlimit parameters

Signed-off-by: Ly Cao <lycao@fb.com>
* changed errorMsg to a literal string instead of static string in main

* moved bogus numeric error to NEXT_UINT32 + add macro NEXT_TSIZE

Signed-off-by: Ly Cao <lycao@fb.com>
Signed-off-by: Ly Cao <lycao@fb.com>
Co-authored-by: Ly Cao <lycao@fb.com>

diff --git a/programs/zstdcli.c b/programs/zstdcli.c
index e93d192060f3573e44325942b0fbc518b0b7756a..583c8a5919b1afe032d24e676d24851036795de7 100644 (file)
--- a/programs/zstdcli.c
+++ b/programs/zstdcli.c
@@ -786,6 +786,18 @@ static unsigned init_nbThreads(void) {
     const char* __nb;             \
     NEXT_FIELD(__nb);             \
     val32 = readU32FromChar(&__nb); \
+    if(*__nb != 0) {         \
+        errorOut("error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed"); \
+    }                             \
+}
+
+#define NEXT_TSIZE(valTsize) {      \
+    const char* __nb;             \
+    NEXT_FIELD(__nb);             \
+    valTsize = readSizeTFromChar(&__nb); \
+    if(*__nb != 0) {         \
+        errorOut("error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed"); \
+    }                             \
 }
 
 typedef enum { zom_compress, zom_decompress, zom_test, zom_bench, zom_train, zom_list } zstd_operation_mode;
@@ -1016,13 +1028,13 @@ int main(int argCount, const char* argv[])
                 if (longCommandWArg(&argument, "--memlimit")) { NEXT_UINT32(memLimit); continue; }
                 if (longCommandWArg(&argument, "--memory")) { NEXT_UINT32(memLimit); continue; }
                 if (longCommandWArg(&argument, "--memlimit-decompress")) { NEXT_UINT32(memLimit); continue; }
-                if (longCommandWArg(&argument, "--block-size=")) { blockSize = readSizeTFromChar(&argument); continue; }
+                if (longCommandWArg(&argument, "--block-size")) { NEXT_TSIZE(blockSize); continue; }
                 if (longCommandWArg(&argument, "--maxdict")) { NEXT_UINT32(maxDictSize); continue; }
                 if (longCommandWArg(&argument, "--dictID")) { NEXT_UINT32(dictID); continue; }
                 if (longCommandWArg(&argument, "--zstd=")) { if (!parseCompressionParameters(argument, &compressionParams)) { badusage(programName); CLEAN_RETURN(1); } continue; }
-                if (longCommandWArg(&argument, "--stream-size=")) { streamSrcSize = readSizeTFromChar(&argument); continue; }
-                if (longCommandWArg(&argument, "--target-compressed-block-size=")) { targetCBlockSize = readSizeTFromChar(&argument); continue; }
-                if (longCommandWArg(&argument, "--size-hint=")) { srcSizeHint = readSizeTFromChar(&argument); continue; }
+                if (longCommandWArg(&argument, "--stream-size")) { NEXT_TSIZE(streamSrcSize); continue; }
+                if (longCommandWArg(&argument, "--target-compressed-block-size")) { NEXT_TSIZE(targetCBlockSize); continue; }
+                if (longCommandWArg(&argument, "--size-hint")) { NEXT_TSIZE(srcSizeHint); continue; }
                 if (longCommandWArg(&argument, "--output-dir-flat")) {
                     NEXT_FIELD(outDirName);
                     if (strlen(outDirName) == 0) {

diff --git a/tests/cli-tests/basic/memlimit.sh b/tests/cli-tests/basic/memlimit.sh
new file mode 100755 (executable)
index 0000000..61a5fc0
--- /dev/null
+++ b/tests/cli-tests/basic/memlimit.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+echo "some data" > file
+
+println "+ zstd --memory=32LB file"
+zstd --memory=32LB file && die "Should not allow bogus suffix"
+println "+ zstd --memory=32LiB file"
+zstd --memory=32LiB file && die "Should not allow bogus suffix"
+println "+ zstd --memory=32A file"
+zstd --memory=32A file && die "Should not allow bogus suffix"
+println "+ zstd --memory=32r82347dn83 file"
+zstd --memory=32r82347dn83 file && die "Should not allow bogus suffix"
+println "+ zstd --memory=32asbdf file"
+zstd --memory=32asbdf file && die "Should not allow bogus suffix"
+println "+ zstd --memory=hello file"
+zstd --memory=hello file && die "Should not allow non-numeric parameter"
+println "+ zstd --memory=1 file"
+zstd --memory=1 file && die "Should allow numeric parameter without suffix"
+rm file.zst
+println "+ zstd --memory=1K file"
+zstd --memory=1K file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+println "+ zstd --memory=1KB file"
+zstd --memory=1KB file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+println "+ zstd --memory=1KiB file"
+zstd --memory=1KiB file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+println "+ zstd --memory=1M file"
+zstd --memory=1M file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+println "+ zstd --memory=1MB file"
+zstd --memory=1MB file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+println "+ zstd --memory=1MiB file"
+zstd --memory=1MiB file && die "Should allow numeric parameter with expected suffix"
+rm file.zst
+
+rm file
+exit 0

diff --git a/tests/cli-tests/basic/memlimit.sh.stderr.exact b/tests/cli-tests/basic/memlimit.sh.stderr.exact
new file mode 100644 (file)
index 0000000..3785b0f
--- /dev/null
+++ b/tests/cli-tests/basic/memlimit.sh.stderr.exact
@@ -0,0 +1,13 @@
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+error: only numeric values with optional suffixes K, KB, KiB, M, MB, MiB are allowed 
+Should allow numeric parameter without suffix
+Should allow numeric parameter with expected suffix
+Should allow numeric parameter with expected suffix
+Should allow numeric parameter with expected suffix
+Should allow numeric parameter with expected suffix
+Should allow numeric parameter with expected suffix
+Should allow numeric parameter with expected suffix

diff --git a/tests/cli-tests/basic/memlimit.sh.stdout.exact b/tests/cli-tests/basic/memlimit.sh.stdout.exact
new file mode 100644 (file)
index 0000000..1821648
--- /dev/null
+++ b/tests/cli-tests/basic/memlimit.sh.stdout.exact
@@ -0,0 +1,13 @@
++ zstd --memory=32LB file
++ zstd --memory=32LiB file
++ zstd --memory=32A file
++ zstd --memory=32r82347dn83 file
++ zstd --memory=32asbdf file
++ zstd --memory=hello file
++ zstd --memory=1 file
++ zstd --memory=1K file
++ zstd --memory=1KB file
++ zstd --memory=1KiB file
++ zstd --memory=1M file
++ zstd --memory=1MB file
++ zstd --memory=1MiB file