Comment by 1718627440

11 days ago

Can someone ELI5 for people who don't know Rust (but e.g. C)?

Rust is supposed to guarantee statically that a reference cannot outlive its referent. The `T: 'static` bound means that the value that Q holds must have a 'static lifetime (e.g. either be an owned value or a reference to a static value). So this program should not compile since a reference to l is not 'static. This code is unsound because rust will drop the string l before q so when the Q destructor runs the underlying string has already been freed yet we have a valid reference to it.

  • So the equivalent in C would be:

            $ cat main.c
            #include <stdlib.h>
            #include <string.h>
    
            struct Q {
                    /*@null@*/ /*@only@*/ char * t;
            };
    
    
            /*@only@*/
            struct Q *
            Q_new (/*@only@*/ char * t)
            {
                    struct Q * q = malloc (sizeof *q);
                    q->t = t;
                    return q;
            }
    
            void
            Q_set (struct Q * q, /*@only@*/ char * t)
            {
                    free (q->t);
                    q->t = t;
                    return;
            }
    
            void
            Q_free (/*@only@*/ struct Q * q)
            {
                    free (q->t);
                    free (q);
                    return;
            }
    
    
    
            int
            main (void)
            {
                    /*@only@*/ char * l;
                    /*@only@*/ struct Q * q;
    
                    l = strdup ("");
                    q = Q_new (NULL);
    
                    Q_set (q, l);
                    free (l);
    
                    /* This is UB */
                    Q_free (q);
    
                    return EXIT_SUCCESS;
            }
    
            $ splint main.c
            Splint 3.1.2 --- 21 Feb 2021
    
            main.c: (in function Q_new)
            main.c:15:3: Arrow access from possibly null pointer q: q->t
              A possibly null pointer is dereferenced.  Value is either the result of a
              function which may return null (in which case, code should check it is not
              null), or a global, parameter or structure field declared with the null
              qualifier. (Use -nullderef to inhibit warning)
               main.c:14:17: Storage q may become null
            main.c: (in function main)
            main.c:43:6: Unrecognized identifier: strdup
              Identifier used in code has not been declared. (Use -unrecog to inhibit
              warning)
            main.c:44:13: Null storage passed as non-null param: Q_new (NULL)
              A possibly null pointer is passed as a parameter corresponding to a formal
              parameter with no /*@null@*/ annotation.  If NULL may be used for this
              parameter, add a /*@null@*/ annotation to the function parameter declaration.
              (Use -nullpass to inhibit warning)
            main.c:47:8: Dead storage l passed as out parameter to free: l
              Memory is used after it has been released (either by passing as an only param
              or assigning to an only global). (Use -usereleased to inhibit warning)
               main.c:46:12: Storage l released
            main.c:12:1: Function exported but not used outside main: Q_new
              A declaration is exported, but not used outside this module. Declaration can
              use static qualifier. (Use -exportlocal to inhibit warning)
               main.c:17:1: Definition of Q_new
            main.c:20:1: Function exported but not used outside main: Q_set
               main.c:25:1: Definition of Q_set
            main.c:28:1: Function exported but not used outside main: Q_free
               main.c:33:1: Definition of Q_free
    
            Finished checking --- 7 code warnings
    

    (The equivalent warning is main.c:47)

    • If you wrote a Q_free function and called it after `free(l);` to actually demonstrate the UAF, that would be pretty close to the semantics of the Rust example.

      4 replies →

  • What does "q.0" and "Some(...)" mean?

    • So Q is a "tuple-struct". In rust you can define a struct like

      struct Q<T> { value: Option<T> }

      with a named field or you can define it like

      struct<T> Q(Some<T>)

      which is the same struct layout but without an explicit name for the value. In that case, the field is implicitly names `0`

      So q.0 = Some(foo) is equivalent to q.value = Some(foo).

      The Some(..) is just one variant of the Option enum:

      enum Option<T> { None, Some(T) }

      Option represents a value that may or may not be initialized. For uninitialized values you use the None variant and then you can initialize it to Some(value).

      The C rough equivalent would be

      struct Q { void* value; }

      struct Q q = q { NULL }; q->value = ...

    • Some(...) is just constructor for an option type as seen in other languages e.g. java's Optional, haskell's Maybe, c++'s std::optional. I assume that it's needed here because the field can't start as a null pointer otherwise. Maybe the vulnerability still works if the field starts with one string value and swaps to a different string value, but having it start holding None made for a simpler demo.

      q.0 is the first field of q, which is anonymous. They could have just as well called it x and written q.x.

    • "Some(...)" means that this Option<T> does have a value of type T in it; "None" means it doesn't.

      "q.0" is not really needed, it's a struct with unnamed fields. Here is the same with named fields:

        struct Q<T: 'static> { pub x: Option<T> }
        ...
        let l = String::new();
        let mut q = Q { x: None };
        q.x = Some(&l);

  • > rust will drop the string l before q

    Why?

    • That's incorrect; the compiler drops q before l. Drops happen in LIFO order; and the compiler knows that it can't drop l before q has been dropped, because q contains a reference to l.

      The issue is that the destructor of q can observe a reference which is about to be deallocated as having a 'static lifetime (i.e. it will live forever), and can thus stash it away somewhere for other code to access after it's been freed (as in the examples in the issue which demonstrate UB.)

    • My thoughts as well. The problem can be alleviated either by making the ownership change an error or "just" invoking the destructors in the correct order.

      But I think https://news.ycombinator.com/item?id=50072252 answers it:

          A 'static lifetime constraint on T says: types that satisfy T must not contain any borrowed value/content whose lifetime will be shorter than the life of the program. 
      

      So the compiler assumes it is guaranteed, that 'l' still exists, so it doesn't consider it in regard of finalizer ordering.

'static lifetime bounds are kind of confusing in Rust, but here goes: A 'static lifetime constraint on T says: types that satisfy T must not contain any borrowed value/content whose lifetime will be shorter than the life of the program. So suppose you have a string s, and Option<&String> q. If q has the constraint 'static, and you do q = Some(&s) this should be rejected by the compiler since a reference to s gets a shorter lifetime than 'static by default (you could do q = Some(&'static s) if s is say a string literal or a leaked box but not for a normal string). In the examples it looks like you have to use a tuple to trick the borrow checker but the concept is the same: Rust allowed violation of the 'static lifetime bounds. The drop implemenation demonstrates it: drop happens in reverse declaration order, so the string s gets dropped first. Now q has a dangling reference, a pointer to dropped memory essentially, and we get a pretty classic use after free bug.