When returning from trusted to untrusted code, we must sanitize the return 
address before taking it.  This ensures that untrusted code cannot use the 
syscall interface to vector execution to an arbitrary address.  This role 
is entrusted to the function NaClSandboxAddr, in sel_ldr.h.  Unfortunately, 
since r572, this function has been a no-op on x86.


-- What happened?

During a routine refactoring, code that once read
  aligned_tramp_ret = tramp_ret & ~(nap->align_boundary - 1);

was changed to read
  return addr & ~(uintptr_t)((1 << nap->align_boundary) - 1);

Besides the variable renames (which were intentional and correct), a shift 
was introduced, treating nap->align_boundary as the log2 of bundle size.  

We didn't notice this because NaCl on x86 uses a 32-byte bundle size.  On 
x86 with gcc, (1 << 32) == 1.  (I believe the standard leaves this behavior 
undefined, but I'm rusty.)  Thus, the entire sandboxing sequence became a 
no-op.

This change had four listed reviewers and was explicitly LGTM'd by two.  
Nobody appears to have noticed the change.


-- Impact

There is a potential for untrusted code on 32-bit x86 to unalign its 
instruction stream by constructing a return address and making a syscall.  
This could subvert the validator.  A similar vulnerability may affect x86-
64.

ARM is not affected for historical reasons: the ARM implementation masks 
the untrusted return address using a different method.