RSA: unchecked PSS salt-length conversion reaches a near-SIZE_MAX write
Testing, LowPublic

Assigned To
Authored By
werner
Tue, Aug 4, 4:46 PM

Description

Unchecked PSS salt-length conversion reaches a near-SIZE_MAX write

RSA-PSS signing parses the bounded salt-length atom without checking its length, parse endpoint, range, or errno, stores it as unsigned int, and passes it to an encoder taking int. On mainstream two's-complement ABIs, 4294967295 becomes -1; mixed signed/unsigned arithmetic then allocates a small buffer before _gcry_randomize receives SIZE_MAX, causing deterministic heap corruption through public gcry_pk_sign.

Vulnerable code

cipher/pubkey-util.c, _gcry_pk_util_data_to_mpi:

c
          s = sexp_nth_data (list, 1, &n);
          if (!s)
            {
              rc = GPG_ERR_NO_OBJ;
              goto leave;
            }
          ctx->saltlen = (unsigned int)strtoul (s, NULL, 10);

cipher/rsa-common.c, _gcry_rsa_pss_encode:

c
_gcry_rsa_pss_encode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
                      int saltlen, int hashed_already,
                      const unsigned char *value, size_t valuelen,
                      const void *random_override)
c
  buflen = 8 + hlen + saltlen + (emlen - hlen - 1);
  buf = xtrymalloc (buflen);
c
        _gcry_randomize (salt, saltlen, GCRY_STRONG_RANDOM);

Why it matters

Services that sign attacker-influenced PSS S-expressions can be terminated and have adjacent heap objects overwritten with random bytes. The public-API ASan reproducer observed a write just beyond a 134-byte allocation, with the call chain ending at _gcry_rsa_pss_encode. FIPS policy may reject oversized salts, but ordinary mode reaches the write; the verification path's separate 16384-byte bound does not protect signing.

Proposed fix

Parse exactly the n atom bytes via a NUL-terminated temporary, with errno = 0, an end pointer, and rejection of empty, trailing, negative, or out-of-range input. Use size_t consistently in the encoder, require saltlen <= emlen - hlen - 2 after checked subtraction, and check every allocation sum. Add regressions for INT_MAX, UINT_MAX, decimal overflow, embedded/trailing bytes, and the modulus-derived boundary.

Revisions and Commits

Event Timeline

werner created this task.
werner created this object with visibility "Public (No Login Required)".
werner created this object with edit policy "Contributor (Project)".
werner renamed this task from pss salt length conversion causes size max write to Unchecked PSS salt-length conversion reaches a near-SIZE_MAX write.Tue, Aug 4, 5:13 PM

My internal comment was: In theory we could check for this but checking the salt length is a duty of the caller. Anyone who creates a s-expression large enough to reach a range limit has other problems in the code than this.

werner lowered the priority of this task from High to Normal.Thu, Aug 6, 9:22 AM
werner added a project: Bug Report.
gniibe renamed this task from Unchecked PSS salt-length conversion reaches a near-SIZE_MAX write to RSA: unchecked PSS salt-length conversion reaches a near-SIZE_MAX write.Tue, Aug 11, 8:49 AM
werner lowered the priority of this task from Normal to Low.Wed, Aug 26, 10:57 AM
werner added a project: Vanity Security.
werner shifted this object from the Restricted Space space to the S1 Public space.

The text looks generated and the description is hard to understand, while some partial texts make sense.

What I do now is:
I review the code from different angle and introduce the salt-length check for different purpose.
As a side effect, the issue claimed in the report will be gone.

gniibe changed the task status from Open to Testing.Mon, Aug 31, 2:47 AM

I pushed rC3ed69d3fb85b: cipher:rsa:pss: Fix SALT-LENGTH handling. as an implementation improvement. It's not directly a response to this ticket, but a side effect.

gniibe mentioned this in Unknown Object (Maniphest Task).Mon, Aug 31, 2:58 AM