Skip to content

[HTTPD] HTTP Request Parsing härten #10

@mgrossmann

Description

@mgrossmann

Summary

httppars.c / httpin.c absichern gegen malformed Requests: Buffer Overflows, fehlende Terminierung, ungültige Methoden. Wichtig für HTTP/1.1.

Kontext

Nach HTTP/1.1 Chunked Transfer Encoding (#51) wurde ein Compliance Test mit h1spec durchgeführt (Deno-basierter Test-Runner, serielle Ausführung, 5s Timeout).

Ergebnis: 21/33 Tests bestanden. Von den 12 Failures sind 8 echte Parsing-Bugs in httpin.c / httppars.c, 3 sind erwartete 501 (kein generischer POST-Handler), 1 ist ein Timeout-Edge-Case.

h1spec Testergebnisse

Bestanden (21/33)

  • Valid GET request (200)
  • Valid GET request with edge cases (Tab in Host, leerer Header-Wert) (200)
  • Empty header value (200)
  • Missing Host header → 400
  • Invalid prefix of request → 501
  • Request with Expect header (200)
  • Alle 15 Fragmentation-Tests (Server wartet korrekt auf vollständigen Request)

Echte Bugs — Header-Validierung fehlt (Kategorie A)

HTTPD gibt 200 statt 400 für ungültige Requests. Betrifft httpin.c Header-Parsing.

Test Ist Soll RFC Fix
Multiple Host headers 200 400 RFC 7230 §5.4 Zählen ob HTTP_HOST mehrfach vorkommt
Non-numeric Content-Length (abc) 200 400 RFC 7230 §3.3.2 Content-Length Wert validieren (nur Ziffern)
Negative Content-Length (-1234) 200 400 RFC 7230 §3.3.2 Content-Length auf nicht-negativ prüfen
Overflowing negative Content-Length 200 400 RFC 7230 §3.3.2 Content-Length Overflow-Prüfung
Invalid header characters ([] in Name) 200 400 RFC 7230 §3.2.6 Header-Name auf gültige token-Zeichen prüfen
Control character in header value (\x07) 200 400 RFC 7230 §3.2.6 Header-Werte auf verbotene Control-Chars prüfen
Invalid line ending (\r ohne \n) 200 400 RFC 7230 §3.5 Strikte CRLF-Prüfung im Header-Parser

Echter Bug — Version nicht validiert (Kategorie C)

Test Ist Soll RFC Fix
Invalid HTTP version (HTTP/9.9) 200 400 oder 505 RFC 7230 §2.6 REQUEST_VERSION auf HTTP/1.0 oder HTTP/1.1 prüfen

Zusätzliche Findings aus Status-Code Testing (Kategorie E)

Test Ist Soll Fix
curl -X PATCH / (unbekannte Methode) 501 405 Method Not Allowed Router/httpd.c: unbekannte Methoden mit 405 statt 501 beantworten
curl mit 8KB URI Connection reset (Worker-Crash) 414 URI Too Long httpin.c: URI-Länge prüfen bevor Buffer-Overflow. Security-Bug!

405: RFC 7231 §6.5.5: 405 ist korrekt wenn die Methode für die Resource nicht erlaubt ist. GET/POST/PUT/HEAD sind implementiert, alles andere → 405.

414: Ein 8KB URI sprengt den Input-Buffer in httpin.c und crasht den Worker. Kritischer Security-Bug. Fix: URI-Länge in httpin.c prüfen und bei Überschreitung sofort 414 senden.

Kein Bug — POST nicht unterstützt (Kategorie B)

Der h1spec Test erwartet einen Echo-Server. HTTPD hat keinen generischen POST-Handler für / — POST geht nur über CGI-Module (mvsMF). Die 501-Antworten sind korrekt.

Test Ist Erwartung des Tests Bewertung
Valid POST with body 501 200 oder 404 Korrekt — kein POST-Handler
Chunked TE POST 501 200 Korrekt — kein POST-Handler
Conflicting TE + CL 501 400 oder 200 Teilweise — 501 statt 400 weil POST

Timeout — Edge Case (Kategorie D)

Test Problem
Request without HTTP version (GET / \r\n\r\n) HTTPD wartet auf Version — Parser erkennt nicht dass keine Version kommt

Implementierung

Alle Fixes in httpin.c, nach dem Header-Parsing-Loop und vor der Transition zu CSTATE_PARSE:

/* --- HTTP/1.1 Request Validation (RFC 7230) --- */

/* 1. Validate HTTP version (§2.6) */
UCHAR *ver = http_get_env(httpc, "REQUEST_VERSION");
if (ver && http_cmp(ver, "HTTP/1.0") != 0 && http_cmp(ver, "HTTP/1.1") != 0) {
    http_resp(httpc, 505); /* HTTP Version Not Supported */
    httpc->state = CSTATE_DONE;
    goto quit;
}

/* 2. Reject multiple Host headers (§5.4) */
/* count occurrences of HTTP_HOST in env array */

/* 3. Validate Content-Length (§3.3.2) */
UCHAR *cl = http_get_env(httpc, "HTTP_CONTENT-LENGTH");
if (cl) {
    /* must be non-negative integer, digits only */
}

/* 4. Validate header field names (§3.2.6) */
/* reject names containing characters outside token set */

/* 5. Validate header values — no control chars (§3.2.6) */

/* 6. Strict CRLF enforcement (§3.5) */

Acceptance Criteria

  • h1spec: Multiple Host headers → 400
  • h1spec: Non-numeric Content-Length → 400
  • h1spec: Negative Content-Length → 400
  • h1spec: Overflowing Content-Length → 400
  • h1spec: Invalid header characters → 400
  • h1spec: Control chars in header value → 400
  • h1spec: Invalid line ending → 400
  • h1spec: Invalid HTTP version (HTTP/9.9) → 400 oder 505
  • curl -X PATCH / → 405 Method Not Allowed (statt 501)
  • curl mit 8KB URI → 414 URI Too Long (statt Connection reset)
  • Bestehende Tests (curl, Zowe CLI, mvsMF) weiterhin funktional
  • h1spec Score: mindestens 29/33 (alle außer POST-Tests und Timeout Edge Case)

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions