Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[scudo] simplify flag parser out of bounds logic #72371

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Conversation

fmayer
Copy link
Contributor

@fmayer fmayer commented Nov 15, 2023

almost NFC, just that now we accept INT_MIN and INT_MAX

as discussed in https://r.android.com/2831100, but I didn't add the *ValueEnd != Value check because I want to keep this change behaviour-keeping.

almost NFC, just that now we accept INT_MIN and INT_MAX

as discussed in https://r.android.com/2831100, but I didn't add the
*ValueEnd != Value check because I want to keep this change
behaviour-keeping.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 15, 2023

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Florian Mayer (fmayer)

Changes

almost NFC, just that now we accept INT_MIN and INT_MAX

as discussed in https://r.android.com/2831100, but I didn't add the *ValueEnd != Value check because I want to keep this change behaviour-keeping.


Full diff: https://github.com/llvm/llvm-project/pull/72371.diff

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/flags_parser.cpp (+9-9)
diff --git a/compiler-rt/lib/scudo/standalone/flags_parser.cpp b/compiler-rt/lib/scudo/standalone/flags_parser.cpp
index 6f9b23ea90e23cc..3d8c6f3789b4ccb 100644
--- a/compiler-rt/lib/scudo/standalone/flags_parser.cpp
+++ b/compiler-rt/lib/scudo/standalone/flags_parser.cpp
@@ -10,6 +10,7 @@
 #include "common.h"
 #include "report.h"
 
+#include <errno.h>
 #include <limits.h>
 #include <stdlib.h>
 #include <string.h>
@@ -143,19 +144,18 @@ bool FlagParser::runHandler(const char *Name, const char *Value,
       break;
     case FlagType::FT_int:
       char *ValueEnd;
+      errno = 0;
       long V = strtol(Value, &ValueEnd, 10);
-      // strtol returns LONG_MAX on overflow and LONG_MIN on underflow.
-      // This is why we compare-equal here (and lose INT_MIN and INT_MAX as a
-      // value, but that's okay)
-      if (V >= INT_MAX || V <= INT_MIN) {
+      if (errno != 0 ||                 // strtol failed (over or underflow)
+          V > INT_MAX || V < INT_MIN || // overflows integer
+          // contains unexpected characters
+          (*ValueEnd != '"' && *ValueEnd != '\'' &&
+           !isSeparatorOrNull(*ValueEnd))) {
         reportInvalidFlag("int", Value);
-        return false;
+        break;
       }
       *reinterpret_cast<int *>(Flags[I].Var) = static_cast<int>(V);
-      Ok =
-          *ValueEnd == '"' || *ValueEnd == '\'' || isSeparatorOrNull(*ValueEnd);
-      if (!Ok)
-        reportInvalidFlag("int", Value);
+      Ok = true;
       break;
     }
     return Ok;

@fmayer fmayer changed the title [[[[[scudo] simplify flag parser out of bounds logic [scudo] simplify flag parser out of bounds logic Nov 15, 2023
@fmayer fmayer merged commit fd8e854 into llvm:main Dec 15, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants