11 pointsby nablags6 hours ago5 comments

quuxplusone3 hours ago
This is why I call string_view a "parameter-only type": it's fine to take as a parameter, but not fine to return or to store as a variable. (For sane definitions of "fine" and "not fine," of course. The blog post demonstrates one way it can go "not fine.")
https://quuxplusone.github.io/blog/2018/03/27/string-view-is...
- ranger_danger3 hours ago
  but when using it as a parameter it IS stored as a variable, just only within that function, right?
dingaling9113 hours ago
Oh this is nasty. But the proposed solution of making admin_id a singleton is weird.
The real fix is don't pass in a temporary to the sanitize function.
stefanos826 hours ago
With GCC and `-fsanitize=address,undefined` flag(s) I have catched it during runtime.
I've got a nice `==31604==ERROR: AddressSanitizer: stack-use-after-scope on address`

  std::string_view username = Sanitize("foobarbazboo");
                                        ^- temporary std::string created here

  std::string_view Sanitize(const std::string& input) { ... }
                 ^                           ^- reference to that temporary string
                 +- returns a view to a temporary string

  std::string_view username = // possibly data freed and dangling string

ranger_danger3 hours ago
> For example, this only works because the string foobarbazboo is the same length as admin_id
12 != 8
I don't get it...
> As you can guess, this is an example of undefined behavior causing unintuitive results.
No I couldn't... and what undefined behavior where? They don't explain anything.
- dingaling9113 hours ago
  He means the string value of the variable admin_id, not the variable name admin_id.
  And the bug comes from the temporary string object created when he passes in the string to Sanitize.
  Sanitize("foobarbaz") -> creates temporary string -> string_view from temporary string -> returns string_view of temporary string
  temporary string destructed
  operations on returned string_view -> oops, use after free.q
  - ranger_danger2 hours ago
    But if Sanitize has a return type of string_view, then returning the temporary variable should make a copy of it (or move it), right? And the string "foobarbazboo" is static so that's not being freed either.
    And the while loop shouldn't even run at all since front() never equals ' '.
    dingaling9112 hours ago
    string_view is literally a reference. Copying a string_view just copies the reference.
    ranger_dangeran hour ago
    But it's not actually taken as a reference (with &) anywhere I see... and even if it's just internally referencing the (static) string, there shouldn't be any issue with that memory either right?
- magackame3 hours ago
  The value of `admin_id` is "root_1231231", which is indeed the same length as "foobarbazboo". Otherwise, yeah, no idea where the bug comes from.