Skip to content

Commit 7b7d80d

Browse files
authored
fix(credentials): strip CR/LF from values to prevent INI injection (#131)
1 parent 03251d7 commit 7b7d80d

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

src/lib/credentials.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,38 @@ describe('serializeCredentials', () => {
8888
expect(text).toContain('api_key = sk');
8989
expect(text).not.toContain('api_url');
9090
});
91+
92+
it('strips newline characters from values to prevent INI injection', () => {
93+
// A malicious apiUrl with embedded newlines could inject new key-value
94+
// pairs or section headers into the credentials file. The serializer
95+
// must strip \n and \r so the written file has exactly one value per
96+
// field and no injected content parsed as separate keys/sections.
97+
const malicious = 'https://evil.com\napi_key = sk-HIJACKED\n[admin]\napi_key = sk-admin';
98+
const text = serializeCredentials({ default: { apiKey: 'sk-real', apiUrl: malicious } });
99+
// The output must NOT contain a standalone [admin] section header
100+
// (it would be on its own line if injection succeeded)
101+
const lines = text.split('\n');
102+
// Only one section header exists: [default]
103+
const sectionHeaders = lines.filter(l => /^\[.+\]$/.test(l.trim()));
104+
expect(sectionHeaders).toEqual(['[default]']);
105+
// Only one api_key line exists (the real one, not an injected duplicate)
106+
const apiKeyLines = lines.filter(l => l.trim().startsWith('api_key'));
107+
expect(apiKeyLines).toHaveLength(1);
108+
expect(apiKeyLines[0]).toContain('sk-real');
109+
// Round-trip: reading back must return only the real key, not the injected one
110+
const parsed = parseCredentials(text);
111+
expect(parsed['default']?.apiKey).toBe('sk-real');
112+
expect(parsed['admin']).toBeUndefined();
113+
});
114+
115+
it('strips \\r\\n (CRLF) injection from values', () => {
116+
const text = serializeCredentials({ default: { apiUrl: 'https://x.com\r\napi_key = pwned' } });
117+
const parsed = parseCredentials(text);
118+
// The injected api_key must NOT be parsed as a real key
119+
expect(parsed['default']?.apiKey).toBeUndefined();
120+
// The api_url value is on one line (newlines stripped)
121+
expect(parsed['default']?.apiUrl).toContain('https://x.com');
122+
});
91123
});
92124

93125
describe('readCredentialsFile / readProfile', () => {

src/lib/credentials.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,15 @@ export function serializeCredentials(file: CredentialsFile): string {
116116
for (const field of fields) {
117117
const value = entry[field];
118118
if (value === undefined || value === '') continue;
119-
lines.push(`${FIELD_TO_FILE_KEY[field]} = ${value}`);
119+
// Guard against INI injection: a value containing newline characters
120+
// would be serialized across multiple lines, allowing an attacker to
121+
// inject arbitrary key-value pairs (or new section headers) into the
122+
// credentials file. A valid API key or URL never contains \n or \r.
123+
// Strip them so a compromised env var or MITM'd backend response
124+
// cannot override the stored api_key on subsequent reads.
125+
const sanitized = value.replace(/[\r\n]/g, '');
126+
if (sanitized === '') continue;
127+
lines.push(`${FIELD_TO_FILE_KEY[field]} = ${sanitized}`);
120128
}
121129
lines.push('');
122130
}

0 commit comments

Comments
 (0)