Audit by Claude Opus 5

[jacqu]

Branche v1

Je vais examiner le code source. Commençons par les fichiers principaux.

J'ai lu la branche v1 : textstore.c/h, ed.c, textedit.c (≈1000 des 1082 lignes) et chacha20_c.c. Je n'ai pas examiné l'assembleur (chacha20.s, liboric.s) ni libscreen.c. Voici ce qui me paraît suspect, par ordre de gravité.

Crypto / sécurité

1. Le mot de passe est utilisé directement comme clé, par répétition. Dans ed_get_password(), un mot de passe de n caractères est complété à 32 en se recopiant lui-même. Pas de KDF, pas de sel, pas d'itérations : « abc » donne une clé « abcabcabc… ». L'entropie réelle reste celle du mot de passe, et la structure périodique réduit encore l'espace de recherche.

2. Oracle de brute-force gratuit. Le magic number 0x94c910ff est chiffré en tête de zone et sert de test de validité du mot de passe. Un attaquant n'a que 4 octets à déchiffrer par candidat — combiné au point 1, le cassage hors-ligne d'un mot de passe court est trivial.

3. Nonce faible et incomplet. textedit_update_nonce() ne remplit que 6 octets sur les 12 attendus par ChaCha20, à partir des compteurs VIA et du timer système. Deux sauvegardes du même fichier à des instants « similaires » après boot peuvent réutiliser le même nonce → XOR des deux versions du texte récupérable. Un compteur monotone stocké dans le fichier serait plus sûr qu'une source d'entropie aussi pauvre.

4. Effet de bord probable sur le VIA. Lire 0x304 (T1C-L) acquitte le flag d'interruption T1 du VIA. Faire ça à chaque sauvegarde peut perturber l'IRQ ROM (clavier, timer). À vérifier sur matériel réel.

5. Aucune authentification. ChaCha20 seul est malléable, et le magic n'est pas un MAC. Voir le point suivant pour la conséquence concrète.

6. Les métadonnées ne sont pas chiffrées. Le chiffrement démarre à &textstore.magic, or dans la struct magic vient après nblines, tlpt[], ptflag[] et lsize[]. Ces champs sont donc écrits en clair sur disque : longueur exacte de chaque ligne, nombre de lignes, pointeurs. Fuite de structure du document, et surtout…

7. …aucune validation au chargement. nblines, lsize[] et les pointeurs tlpt[] sont repris tels quels du fichier, sans borne ni vérification, et sans intégrité. Un fichier .ted modifié (même sans connaître la clé, puisque cette zone est en clair) donne des pointeurs arbitraires → écritures n'importe où en RAM. C'est à mon avis le bug le plus sérieux du projet.

8. Effacement mémoire incomplet. textedit_exit() fait memset(&textstore, 0, textstore_sizeof()), or textstore_sizeof() s'arrête au pointeur le plus haut encore utilisé. Les emplacements libérés au-dessus gardent leur texte en clair. Idem si on sort par ed_fatal_error("INVALID PASSWORD") : rien n'est effacé.

Corruption mémoire

9. textstore_fix_pointers() — boucle mal bornée. La recherche du plus petit index utilisé parcourt ptflag[] de 0 à nblines, alors que les emplacements occupés peuvent tous se situer au-delà de nblines (c'est le cas après des insertions/suppressions). Dans ce cas la boucle sort avec i == nblines et l'offset calculé est faux → tous les pointeurs de ligne deviennent invalides. Il faudrait balayer jusqu'à TEXTSTORE_LINES_MAX. Accessoirement, si nblines == 0, address reste à 0xffff et l'offset est absurde.

10. textstore_del_chars() — longueur de memmove erronée. La longueur utilisée est lsize - char_nb - 1 au lieu de lsize - char_nb - chars_nb. Le résultat visible reste correct (le memset final recouvre les octets parasites), mais la lecture déborde jusqu'à chars_nb - 1 octets au-delà de la fin de ligne — donc hors de buf pour le dernier emplacement du buffer.

11. textstore_sizeof() peut déborder. Si nblines == 0, la boucle laisse j = 0, puis j += LINE_SIZE et la soustraction j - (uint16_t)&textstore passe en négatif → taille énorme utilisée ensuite comme longueur de chiffrement et de memset.

12. Les garde-fous n'existent qu'en debug. Tous les contrôles de bornes de textstore.c sont sous #ifdef ED_DEBUG. En release, textstore_write_chars() et consorts écrivent sans vérifier line_nb. Combiné au point 7, c'est ce qui rend le chargement d'un fichier corrompu dangereux.

13. Récursion de textedit_event(). La fonction s'appelle elle-même dans plusieurs branches : CTRL-Z (jusqu'à TABSZ niveaux), le word-wrap (textedit_event(c)), ESC → CTRL-S, flèche droite → flèche bas. Sur 6502 avec la pile logicielle cc65, l'empilement combiné (tabulation en fin de ligne pleine déclenchant un wrap, par exemple) mérite d'être mesuré.

Logique d'édition

14. Curseur en colonne LINE_SIZE. Sur page down (CTRL-B) et sur flèches haut/bas, le curseur est positionné à lsize[lpntr], qui peut valoir TEXTSTORE_LINE_SIZE sur une ligne pleine — donc une colonne hors écran. CTRL-X et la flèche gauche font ce clamp (cur_x-- / LINE_SIZE - 1), pas les autres. Incohérence probable.

15. nblines - 1 en arithmétique non signée. ARROW_DOWN et CTRL-X testent lpntr >= textstore.nblines - 1. Si nblines valait 0 (fichier corrompu, cf. point 7), l'expression vaut 65535 et les tests s'inversent.

16. Effacement bloqué sur un mot trop long. Dans KEY_DEL, si aucune espace de la ligne ne permet une fusion avec la ligne précédente, i finit à 0, rien n'est déplacé, et l'appui sur DEL ne fait que remonter le curseur. L'utilisateur ne peut plus supprimer.

Divers

17. Pointeur passé à %u. Le LOAD de textedit_init() passe &textstore à un %u, alors que le SAVEU fait bien le (uint16_t). Ça marche par chance en cc65, c'est du comportement indéfini.

18. snprintf avec une taille de LIBSCREEN_NB_COLS. Dans textedit_status_print/popup, la ligne d'état est remplie d'espaces puis tronquée par un \0 au milieu, qui est ensuite recopié à l'écran comme un caractère de contrôle Oric. Ailleurs (CTRL-G) la borne est NB_COLS+1 : les deux conventions coexistent.

19. rand() sans srand(). L'écran de veille produit exactement la même séquence à chaque lancement.

20. E de SAVEU probablement inclusif. L'adresse de fin vaut &textstore + textstore_sizeof(), ce qui sauvegarde un octet de plus que nécessaire sous Sedoric. Sans conséquence, mais à confirmer.

Si tu veux, je peux détailler un correctif concret pour les points 9, 10 et 7 — ce sont les trois qui me semblent mériter un patch en priorité.

Amélioration crypto

Bonne nouvelle : vous avez déjà tout ce qu'il faut. ChaCha20 en assembleur optimisé est un excellent KDF — il suffit de le boucler sur lui-même. Aucun nouveau primitif, aucune table, ~30 octets de code.

Point 1 — dérivation itérée

#define ED_KDF_ROUNDS  128        // ~9 s sur Atmos 1 MHz
#define ED_KEY_SZ      32

uint8_t ed_key[ED_KEY_SZ];        // 32 octets de RAM

void ed_derive_key( char *password, uint8_t *salt ) {
  uint8_t i;
  // Graine : mot de passe complété par des zéros (plus de répétition)
  memset( ed_key, 0, ED_KEY_SZ );
  strncpy( (char*)ed_key, password, ED_KEY_SZ );
  // key <- key XOR ChaCha20(key, salt, i)
  for ( i = 0; i < ED_KDF_ROUNDS; i++ ) {
    chacha_process( ed_key, ED_KEY_SZ, ed_key, salt, i );
  }
}

Puis on remplace (uint8_t*)textedit_password par ed_key aux trois appels de chacha_process, et on efface password juste après la dérivation.

Deux détails :

• Le fait que data et key soient le même buffer donne nouvelle_clé = ancienne ⊕ ChaCha(ancienne) — un feed-forward à la Davies-Meyer, non inversible, c'est exactement ce qu'on veut. Mais vérifiez que chacha20.s charge bien la clé dans l'état avant de XORer les données. Sur 32 octets c'est un seul bloc, donc a priori oui.
• Le compteur i fait varier le keystream à chaque tour.

Sel : 12 octets stockés en clair avec le nonce, tirés une seule fois à la création du fichier (pas à chaque sauvegarde, sinon il faut redériver à chaque CTRL-S). Ça casse la compatibilité de format — vous avez déjà textstore_fix_pointers, donc à intégrer là. C'est la partie optionnelle si les octets manquent : sur une machine mono-utilisateur avec quelques fichiers, le sel apporte peu.

Coût total : 32 (+12) octets de RAM, ~9 s une seule fois au chargement. Réglable via ED_KDF_ROUNDS (64 → ~4,5 s).

Point 2 — ne dépensez rien dessus

Retirer le magic number ne vous achète pratiquement rien. Votre texte en clair est déjà un distingueur : l'attaquant déchiffre une ligne et teste si les octets tombent dans la plage ASCII imprimable. Ça lui coûte le même bloc ChaCha, et le magic ne lui économise que quelques comparaisons. En revanche, le supprimer vous coûte un vrai risque : l'utilisateur qui se trompe de mot de passe voit du charabia, sauve, et détruit son fichier.

Gardez-le. Une fois le KDF en place, le coût par essai est dominé par les 128 tours, pas par la vérification.

Le levier à zéro octet

Soyons honnêtes sur ce que le KDF achète : 128 tours ≈ 8 Ko de ChaCha par essai, soit ~3 µs sur un CPU moderne. Environ 7 bits de travail supplémentaire. Un mot de passe de 8 lettres minuscules tombe toujours en quelques jours.

Ce qui protège réellement, c'est l'entropie saisie. Vos 32 caractères permettent une passphrase de 5–6 mots (~65-77 bits) — hors de portée, KDF ou pas. Et à cette longueur, le padding par répétition ne s'appliquait déjà plus.

Donc : imposez un minimum dans ed_get_password() (12 caractères ?) et changez le prompt de "pwd: " en quelque chose comme "phrase: ". Ça coûte une comparaison et zéro octet de données, et c'est de loin le plus rentable des trois changements.

Branche v2

J'ai lu toute la v2 (textstore.c 632 l., textedit.c 1179 l., ed.c, libscreen.c, liboric_c.c, chacha20.s, headers, Makefile). La réécriture autour de textedit_insert() change beaucoup de choses. Voici la mise à jour.

Ce qui est corrigé depuis la v1

• #12 — ED_DEBUG et ED_VERBOSE sont maintenant actifs dans ed.h, les garde-fous existent en build par défaut.
• #13 — la récursion de textedit_event() est très réduite (CTRL-Z passe par textedit_insert).
• #16 — l'effacement bloqué sur un mot trop long disparaît avec le nouveau moteur de reflow.
• Nouveau et bon : le test GRAB au démarrage (0xA3 == 0x97), la restauration du charset dans textedit_exit(), la reprise après pause d'impression.

Toujours présents (v1 → v2, code inchangé)

#9 textstore_fix_pointers() — la 2ᵉ boucle balaie toujours ptflag[] de 0 à nblines au lieu de TEXTSTORE_LINES_MAX. Si tous les emplacements occupés sont au-delà de nblines, l'offset est faux et tous les pointeurs deviennent invalides.

#10 textstore_del_chars() — ligne 304, la longueur du memmove est toujours lsize - char_nb - 1 au lieu de - chars_nb.

#11 textstore_sizeof() — pas de garde sur nblines == 0.

#14 curseur en colonne 40 — CTRL_B, CTRL_V et les flèches haut/bas posent cur_x = lsize[lpntr], qui vaut 40 sur ligne pleine. textedit_cursor_refresh() calcule alors textbuf[27*40+40] = 1120 → écriture un octet après l'écran (0xBFE0). Le contrôle cur_x >= LINE_SIZE existe dans textedit_insert() mais pas sur ces chemins-là.

#17, #18, #19, #20 — %u sur &textstore (l. 198), snprintf(status, NB_COLS, …), rand() sans srand(), adresse E du SAVEU.

Crypto (#1 à #8) — inchangée. ed_get_password() remplit toujours par répétition, textedit_update_nonce() remplit toujours 6 octets sur 12, le chiffrement démarre toujours à &textstore.magic donc nblines/lsize[]/tlpt[] restent en clair. Le KDF dont on a parlé n'est pas là.

Régressions et bugs nouveaux

A. CTRL-Z insère le caractère 26, pas des espaces. Ligne 345 :

if ( textedit_insert( textedit_lpntr, textedit_cur_x, c ) == false )

c vaut ici TEXTEDIT_CTRL_Z (26). En v1 c'était textedit_event( LIBSCREEN_SPACE ). La tabulation insère donc des caractères de contrôle dans le texte. Correctif : passer TEXTSTORE_CHAR_SPACE.

B. Le chemin rapide de textedit_insert() est mort — et c'est le cas d'usage principal. Lignes 916-935, la condition exige simultanément cpos < lsize[lpos] - 1 et, dans la 1ʳᵉ sous-branche, cpos == lsize[lpos]. Contradictoire. Résultat : taper en fin de ligne emprunte toujours le chemin lent (reconstruction du linebuf, reflow, textedit_screen_refresh() complet à 35 ms). C'est-à-dire la frappe normale. À 1 MHz ça se voit. Il faut sortir le test « fin de texte » du && :

bool at_end = ( lpos == textstore.nblines - 1 ) && ( cpos == textstore.lsize[lpos] );
bool inside = ( cpos > 0 ) && ( cpos < textstore.lsize[lpos] - 1 );
if ( ( at_end || inside ) && room_left && c != DEL && c != RET && c != SPACE ) { … }

C. Lecture à l'index −1 dans textstore_move_first_words_up(). Ligne 590 déréférence tlpt[line_nb-1][lsize[line_nb-1] - 1] avant le test « ligne précédente vide » de la ligne 595. Si lsize == 0, lsize-1 est promu en int → −1 → lecture de l'octet précédant la ligne. Il suffit d'intervertir les deux blocs.

D. textstore_reformat() utilise des indices périmés. Ligne 563, tlpt[j][lsize[j]-1] est lu juste après textstore_move_first_words_up(j), qui peut avoir appelé textstore_del_line(j) ou del_line(j-1). Deux conséquences : si j était la dernière ligne, j == nblines maintenant et tlpt[j] est un pointeur mort ; et si une ligne a été supprimée, le ++j de la ligne 567 saute la ligne qui a pris sa place. Il faut relire nblines et ne pas incrémenter j après une suppression.

E. Perte de texte si la mémoire sature pendant une insertion. Dans la boucle de reflow (l. 1034-1115), textstore_clear_line() a déjà vidé les lignes cibles ; si textstore_insert_line() échoue au milieu, la fonction fait return false et les mots restants du linebuf ne sont jamais réécrits. L'utilisateur perd jusqu'à 3 lignes de texte. Il faudrait tester la place disponible avant de vider quoi que ce soit.

F. Les garde-fous détectent sans empêcher. Trois sites utilisent printf("E1"/"E2"/"E3") au lieu de ed_fatal_error() en mode non-verbeux, puis continuent. Le plus gênant est le E3 (l. 963-969) : il détecte le débordement de linebuf… et le memcpy s'exécute quand même juste après. Même chose pour E2 (filename == NULL puis snprintf sur NULL) et E1 (aucune ligne allouée puis usage).

G. libscreen_copyline_inv() modifie son argument. Le XOR est fait en place sur b[]. Appelée sur des littéraux (l. 380, 395-396), elle écrit dans les chaînes constantes. En cc65 la RODATA est en RAM donc ça passe, mais la barre du bas de l'aide alterne inversée/normale à chaque affichage. Un buffer temporaire, ou un XOR à la volée pendant la copie, règle ça.

H. sei pendant toute l'opération ChaCha. chacha20.s masque les interruptions de l'entrée à la sortie. Sur un document plein (~15,4 Ko à 952 o/s) ça fait ~16 s IRQ coupées : plus de scan clavier, plus de mise à jour du timer ULA (0x276) — celui-là même qui alimente le nonce. Si c'est délibéré, une note dans le source aiderait ; sinon, un cli/sei par bloc de 64 octets coûterait quelques cycles et rendrait la machine réactive.

I. Un seul del_line après reflow. Ligne 1118, if ( lidx < lidxstop - 1 ) textstore_del_line( lidx + 1 ); ne supprime qu'une ligne, alors que le reflow de 3 lignes peut en libérer deux. Il reste une ligne vide au milieu du texte.

J. Détails mineurs. textstore_insert_line() l. 1171 : retour ignoré. Statut : le \0 de snprintf atterrit en colonne 39 et est envoyé à l'écran comme code attribut « encre noire ». lpos == 0 donne lidxstop = INSBUFSCAN - 1 = 2, donc 2 lignes scannées au lieu de 3 — asymétrie probablement involontaire.

Sur votre contrainte mémoire

ED_VERBOSE embarque __FILE__ pour chaque site d'appel plus un fprintf("%s:%lu"), ce qui tire le formatage long. Vous récupérez sans doute quelques centaines d'octets — exactement ce qui vous manque — en gardant ED_DEBUG seul avec les codes courts ("S0", "E4"…). Et ED_DEBUG coûte aussi du CPU sur le chemin chaud : votre propre commentaire dans libscreen.c note 16 ms → 18 ms sur copyline.

Priorité si vous ne deviez traiter que trois points : B (performance de frappe), E (perte de données) et A (tabulation cassée). C et D ensuite, ce sont les deux accès hors bornes réels du nouveau moteur.

Audit chacha20

Pas de problème majeur détecté