Bug: `binascii.a2b_uu` incorrectly assumes padded bytes are always whitespace

Ouverte
#100,308 6 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Évaluation

Difficulté
3/5
Temps estimé
1-2 jours
Accessibilité débutants
52/100
Type d'issue
Bug
Clarté
Plutôt claire
Activité
Calme
Stack technique
c, python
Domaine
backend

Piste de recherche

Commencez par la validation du padding dans Modules/binascii.c autour des lignes liées, puis examinez le workaround dans Lib/encodings/uu_codec.py. Reproduisez l’exemple b'%-@ !' et comparez le comportement avec les implémentations de UU-decoder liées ; le travail est terminé lorsque du padding valide qui n’est pas constitué d’espaces blancs est décodé comme prévu sans perturber la gestion existante des espaces blancs.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

extension-modules
Bug Description

I was decoding some UUEncoded data when I encountered a 'Trailing Garbage' error from the binascii.a2b_uu function. After digging into Linux's uu decode implementation(L248) and other resources (linked below) I'm decently certain the python implementation is bugged.

The following is what I tried:
from binascii import a2b_uu
s = '%-@     !'
decoded = a2b_uu(s)
The expected output is:
print(decoded)  # b'6\x00\x00\x00\x00'
The actual output is:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
binascii.Error: Trailing garbage

Notice there are 5 bytes in the expected output (b'6\x00\x00\x00\x00') because the % (first byte of input string, s) means 5 bytes of data follow (ascii code 37 - 32 = 5). UUEncoding requires output be divisible by 3 bytes so an extra padding character is added. In this case it's an !.

The python implementation assumes the padding is always whitespace. Different uuencoders will use different characters for padding though. I've seen three so far: , `, and !.

The following several lines of code are the issue

Proposed fix

Simply remove the following lines (279 - 296). Or if we really want the verification of padding we can include the '!' in the condition of valid padding chars. (The linked linux implementation does not verify padding, however.) And based on my research, there isn't a well defined padding character so we will be jumping to the same potentially false conclusion that we have here: believing we've accounted for all the padding characters that exist in the wild.

/*
** Finally, check that if there's anything left on the line
** that it's whitespace only.
*/
while( ascii_len-- > 0 ) {
    this_ch = *ascii_data++;
    /* Extra '`' may be written as padding in some cases */
    if ( this_ch != ' ' && this_ch != ' '+64 &&
         this_ch != '\n' && this_ch != '\r' ) {
        state = get_binascii_state(module);
        if (state == NULL) {
            return NULL;
        }
        PyErr_SetString(state->Error, "Trailing garbage");
        Py_DECREF(rv);
        return NULL;
    }
}

Problematically, this bug propagated up to the uu_codec decode implementation as well. See the following code

A comment indicates the caught exception and "workaround" are due to broken uuencoders. According to what I've read, it's the broken python binascii.a2b_uu that incorrectly assumes any padding bytes are or `.

Here are the sources for my understanding of uu encoding:
Examples of non whitespace padding
Wikipedia uuencoding
Busybox uudecode implementation

Following is an illustration that helped me find a sense of understanding:
uuencode-bug-explanation

[1] I couldn't find an RFC or other standards document so I looked for the earliest implementation I could find (1983 Linux implementation) along with the wikipedia entry.

In the meantime

If others encounter this issue I'm using the following workaround:

import binascii
from binascii import a2b_uu
from io import BytesIO

my_bytes = BytesIO()
line_bytes = b'%-@     !'
line = line_bytes.decode(encoding='ascii')
try:
    my_bytes.write(a2b_uu(line))
except binascii.Error as err:
    if 'trailing garbage' in str(err).lower():
        n_bytes = line_bytes[0] - 32
        assert n_bytes <= 45 and n_bytes <= len(line[1:])
        workaround_line = f'M{line[1:]}'  # replace first byte of UUEncoded line with max length specifier (M)
        data = a2b_uu(workaround_line)[:n_bytes]
        my_bytes.write(data)
    else:
        raise err
Langage dominant
Python
Étoiles
77.2k
Forks
36k
Merge moyen
1 j 9 h
PR mergées (30 j)
558

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de python/cpython

Toutes les issues de python/cpython

Issues similaires

Plus d'issues Python

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.