Hacker News

nablags

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_danger2 hours ago

but when using it as a parameter it IS stored as a variable, just only within that function, right?

dingaling9112 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.

stefanos825 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`

blastonico2 hours ago

  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_danger2 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.

dingaling9112 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_dangeran hour 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 ' '.

dingaling911an hour ago

string_view is literally a reference. Copying a string_view just copies the reference.

ranger_danger25 minutes 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?

magackame2 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.

source