Skip to content

Replace heuristic printable checker with simple UTF-8/ASCII validator #2245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 28, 2025
+25 −107
Merged

Replace heuristic printable checker with simple UTF-8/ASCII validator #2245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 1 addition & 61 deletions src/AudioTools/Communication/HTTP/HttpLineReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,67 +72,7 @@ class HttpLineReader {
}
str[result - 1] = 0;
if (is_buffer_overflow) {
// SAFETY FIX: Don't print potentially corrupted binary data with %s
// Binary garbage can contain terminal escape codes or invalid UTF-8 that crashes Serial.printf()
//
// This fix prevents ESP32 hard resets when HTTP servers send malformed headers
// Real-world trigger: http://fast.citrus3.com:2020/stream/wtmj-radio
//
// Strategy:
// 1. Sanitize the actual buffer (prevents parser poisoning downstream)
// 2. Count printable vs binary content
// 3. Use hex dump for binary garbage (safer than string masking)
// 4. Limit output to 256 bytes (prevents log spam)

int printable = 0;
int non_printable = 0;
int actual_len = 0;

// First pass: count and find actual length
for (int i = 0; i < len && str[i] != 0; i++) {
actual_len = i + 1;
if (str[i] >= 32 && str[i] <= 126) {
printable++;
} else if (str[i] != '\r' && str[i] != '\n' && str[i] != '\t') {
non_printable++;
// CRITICAL: Sanitize the actual buffer to prevent parser poisoning
// Replace binary garbage with space to avoid confusing HTTP header parser
str[i] = ' ';
}
}

// Limit logging output to 256 bytes to prevent excessive serial spam
int log_len = (actual_len > 256) ? 256 : actual_len;

// If mostly binary garbage (>50% non-printable), use hex dump for safety
if (non_printable > printable) {
LOGE("Line cut off: [%d bytes, %d binary chars - showing hex dump of first %d bytes]",
actual_len, non_printable, (log_len > 32 ? 32 : log_len));

// Hex dump (safer than string output - never misinterpreted)
// Show first 32 bytes maximum
int hex_len = (log_len > 32) ? 32 : log_len;
for (int i = 0; i < hex_len; i += 16) {
char hex_line[64];
int line_len = (hex_len - i > 16) ? 16 : (hex_len - i);
int pos = 0;
for (int j = 0; j < line_len; j++) {
pos += snprintf(hex_line + pos, sizeof(hex_line) - pos, "%02X ", (uint8_t)str[i + j]);
}
LOGE(" %04X: %s", i, hex_line);
}
} else {
// Mostly printable - safe to log as string (already sanitized in-place above)
// Truncate to 256 bytes for logging
if (log_len < actual_len) {
char saved = str[log_len];
str[log_len] = 0;
LOGE("Line cut off: %s... [%d more bytes]", str, actual_len - log_len);
str[log_len] = saved;
} else {
LOGE("Line cut off: %s", str);
}
}
LOGE("HttpLineReader %s", "readlnInternal->cut off too long line");
}

return result;
Expand Down
70 changes: 24 additions & 46 deletions src/AudioTools/CoreAudio/AudioMetaData/MetaDataICY.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#pragma once
#include <ctype.h> //isascii
#include "AudioToolsConfig.h"
#include "AbstractMetaData.h"
#include "AudioTools/CoreAudio/AudioBasic/StrView.h"
#include "AudioTools/Communication/HTTP/AbstractURLStream.h"

#ifndef AUDIOTOOLS_METADATA_ICY_ASCII_ONLY
#define AUDIOTOOLS_METADATA_ICY_ASCII_ONLY true
#endif

namespace audio_tools {

/**
Expand Down Expand Up @@ -109,12 +112,7 @@ class MetaDataICY : public AbstractMetaData {
metaDataLen = metaSize(ch);
LOGI("metaDataLen: %d", metaDataLen);
if (metaDataLen > 0) {
// Enhanced validation: reject suspiciously large metadata (>4080 bytes = 255*16)
// Also reject extremely small metadata that's unlikely to be valid
if (metaDataLen > 4080 || metaDataLen < 16) {
LOGW("Suspicious metaDataLen %d -> skipping metadata block", metaDataLen);
nextStatus = ProcessData;
} else if (metaDataLen > 200) {
if (metaDataLen > 200) {
LOGI("Unexpected metaDataLen -> processed as data");
nextStatus = ProcessData;
} else {
Expand Down Expand Up @@ -170,46 +168,28 @@ class MetaDataICY : public AbstractMetaData {
/// determines the meta data size from the size byte
virtual int metaSize(uint8_t metaSize) { return metaSize * 16; }

inline bool isAscii(uint8_t ch){ return ch < 128;}

/// Make sure that the result is a valid ASCII string with printable characters
/// Enhanced validation to reject corrupted metadata before it affects audio stream
virtual bool isAscii(char* result, int l) {
if (l < 1) return false;

// Check entire metadata string, not just first 10 characters
int printable_count = 0;
int control_count = 0;

/// Make sure that the result is a printable string
virtual bool isPrintable(const char* str, int l) {
int remain = 0;
for (int j = 0; j < l; j++) {
uint8_t ch = (uint8_t)result[j];

// Reject non-ASCII bytes (>= 128)
if (ch >= 128) return false;

// Count printable vs control characters
if (ch >= 32 && ch <= 126) {
printable_count++;
} else if (ch == '\n' || ch == '\r' || ch == '\t' || ch == 0) {
// Allow common control characters
continue;
uint8_t ch = str[j];
if (remain) {
if (ch < 0x80 || ch > 0xbf) return false;
remain--;
} else {
// Unusual control character
control_count++;
if (ch < 0x80) { // ASCII
if (ch != '\n' && ch != '\r' && ch != '\t' && ch < 32 || ch == 127)
return false; // control chars
}
#if !AUDIOTOOLS_METADATA_ICY_ASCII_ONLY
else if (ch >= 0xc2 && ch <= 0xdf) remain = 1;
else if (ch >= 0xe0 && ch <= 0xef) remain = 2;
else if (ch >= 0xf0 && ch <= 0xf4) remain = 3;
#endif
else return false;
}
}

// Require at least 50% printable characters to reject binary garbage
// 50% threshold is the absolute minimum - accepts any ICY padding strategy
// Binary garbage typically has < 30% printable, so 50% provides good separation
// Super CFL: 68.8% (33/48) easily passes
if (printable_count < (l * 0.50)) {
LOGW("Metadata validation failed: only %d/%d printable (%.1f%%)",
printable_count, l, (printable_count * 100.0) / l);
return false;
}

return true;
return remain == 0;
}

/// allocates the memory to store the metadata / we support changing sizes
Expand All @@ -226,8 +206,7 @@ class MetaDataICY : public AbstractMetaData {
// CHECK_MEMORY();
TRACED();
metaData[len] = 0;
// Use full validation on entire metadata string, not just first 12 bytes
if (isAscii(metaData, len)) {
if (isPrintable(metaData, len)) {
LOGI("%s", metaData);
StrView meta(metaData, len + 1, len);
int start = meta.indexOf("StreamTitle=");
Expand All @@ -247,7 +226,6 @@ class MetaDataICY : public AbstractMetaData {
// Don't print corrupted binary data - could contain terminal control codes
LOGW("Unexpected Data: corrupted metadata block rejected (len=%d)", len);
// Signal corruption to application so it can disconnect/reconnect
// This is critical: metaint boundary is now desynchronized and audio will glitch
if (callback != nullptr) {
callback(Corrupted, nullptr, len);
}
Expand Down