Skip to content

Code base Improvement #2125

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

Open
JohnKimaiyo wants to merge 1 commit into
base: master
Choose a base branch
from
+71 −171
Open

Code base Improvement #2125

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
242 changes: 71 additions & 171 deletions src/addagent/b64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,202 +24,102 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define TRUE 1
#define FALSE 0
#include <stdbool.h> // For using bool instead of int for boolean functions

/* Prototypes */
static int is_base64(char c);
static char encode(unsigned char u);
static unsigned char decode(char c);
static bool is_base64(char c);
static char encode_char(unsigned char u);
static unsigned char decode_char(char c);

/* Global variables */
/* Global functions */
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "Global functions" but these are function prototypes/declarations, not global variables. While technically functions have global scope by default, the more conventional comment would be "Function prototypes" or "Public API" since these appear to be the public interface functions. The original comment "Global variables" was indeed incorrect, but "Global functions" is still somewhat misleading.

Suggested change
/* Global functions */
/* Public API (function prototypes) */

Copilot uses AI. Check for mistakes.
char *decode_base64(const char *src);
char *encode_base64(int size, char *src);


/* Base64 encode and return size data in 'src'. The caller must free the
* returned string.
* Returns encoded string otherwise NULL
*/
char *encode_base64(int size, char *src)
{
int i;
char *out, *p;

if (!src) {
return NULL;
}

if (!size) {
size = strlen((char *)src);
}

out = (char *)calloc(sizeof(char), size * 4 / 3 + 4);
if (!out) {
return NULL;
}

p = out;
char *encode_base64(int size, const char *src);

/* Encode a character into Base64 */
static char encode_char(unsigned char u) {
if (u < 26) return 'A' + u;
if (u < 52) return 'a' + (u - 26);
if (u < 62) return '0' + (u - 52);
return (u == 62) ? '+' : '/';
}

for (i = 0; i < size; i += 3) {
unsigned char b1 = 0, b2 = 0, b3 = 0, b4 = 0, b5 = 0, b6 = 0, b7 = 0;
/* Decode a Base64 character */
static unsigned char decode_char(char c) {
if (c >= 'A' && c <= 'Z') return c - 'A';
if (c >= 'a' && c <= 'z') return c - 'a' + 26;
if (c >= '0' && c <= '9') return c - '0' + 52;
return (c == '+') ? 62 : 63;
}

b1 = src[i];
/* Check if a character is a valid Base64 character */
static bool is_base64(char c) {
return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') ||
(c >= '0' && c <= '9') || (c == '+') ||
(c == '/') || (c == '=');
}

if (i + 1 < size) {
b2 = src[i + 1];
}
/* Encode data to Base64 */
char *encode_base64(int size, const char *src) {
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature has been changed to use const char *src parameter, but the header files at src/addagent/manage_agents.h:18 and src/win32/ui/os_win32ui.h:50 still declare it with char *src (non-const). This creates a signature mismatch between the declaration and definition. You should update the header files to match this new signature with const char *src, otherwise this may cause compilation warnings or errors depending on the compiler settings.

Copilot uses AI. Check for mistakes.
if (!src) return NULL;

if (i + 2 < size) {
b3 = src[i + 2];
}
if (size == 0) size = strlen(src);

b4 = b1 >> 2;
b5 = ((b1 & 0x3) << 4) | (b2 >> 4);
b6 = ((b2 & 0xf) << 2) | (b3 >> 6);
b7 = b3 & 0x3f;
int out_size = (size + 2) / 3 * 4 + 1; // +1 for null terminator
char *out = (char *)calloc(out_size, sizeof(char));
if (!out) return NULL;

*p++ = encode(b4);
*p++ = encode(b5);
char *p = out;

if (i + 1 < size) {
*p++ = encode(b6);
} else {
*p++ = '=';
}
for (int i = 0; i < size; i += 3) {
unsigned char b1 = src[i];
unsigned char b2 = (i + 1 < size) ? src[i + 1] : 0;
unsigned char b3 = (i + 2 < size) ? src[i + 2] : 0;

if (i + 2 < size) {
*p++ = encode(b7);
} else {
*p++ = '=';
}
*p++ = encode_char(b1 >> 2);
*p++ = encode_char(((b1 & 0x3) << 4) | (b2 >> 4));
*p++ = (i + 1 < size) ? encode_char(((b2 & 0xf) << 2) | (b3 >> 6)) : '=';
*p++ = (i + 2 < size) ? encode_char(b3 & 0x3f) : '=';
}

Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null terminator is not explicitly written to the output buffer. While calloc initializes memory to zero (which provides a null terminator), after the loop completes, the pointer p has advanced beyond the last written character but the null terminator position is not guaranteed. For Base64 encoding, the output should always be a multiple of 4 characters (with padding), so the calculation (size + 2) / 3 * 4 + 1 should be correct, but it would be more explicit and maintainable to write *p = '\0'; after the loop to document the intent and ensure null termination regardless of calloc behavior.

Suggested change
*p = '\0';

Copilot uses AI. Check for mistakes.
return out;
}

/* Decode the base64 encoded string 'src' into the memory pointed to by
* 'dest'. The dest buffer is NUL terminated.
* Returns NULL in case of error
*/
char *decode_base64(const char *src)
{
if (src && *src) {
char *dest;
unsigned char *p;
int k, l = strlen(src) + 1;
unsigned char *buf;

/* The size of the dest will always be less than the source */
dest = (char *)calloc(sizeof(char), l + 13);
if (!dest) {
return (NULL);
}

p = (unsigned char *)dest;

buf = (unsigned char *) malloc(l);
if (!buf) {
free(dest);
return (NULL);
}

/* Ignore non base64 chars as per the POSIX standard */
for (k = 0, l = 0; src[k]; k++) {
if (is_base64(src[k])) {
buf[l++] = src[k];
}
}

for (k = 0; k < l; k += 4) {
char c1 = 'A', c2 = 'A', c3 = 'A', c4 = 'A';
unsigned char b1 = 0, b2 = 0, b3 = 0, b4 = 0;

c1 = buf[k];

if (k + 1 < l) {
c2 = buf[k + 1];
}

if (k + 2 < l) {
c3 = buf[k + 2];
}

if (k + 3 < l) {
c4 = buf[k + 3];
}

b1 = decode(c1);
b2 = decode(c2);
b3 = decode(c3);
b4 = decode(c4);
/* Decode Base64 encoded string */
char *decode_base64(const char *src) {
if (!src || !*src) return NULL;

*p++ = ((b1 << 2) | (b2 >> 4) );
int len = strlen(src);
int out_len = len * 3 / 4;
char *dest = (char *)calloc(out_len + 1, sizeof(char)); // +1 for null terminator
Comment on lines +92 to +93
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memory allocation calculation len * 3 / 4 may be insufficient for the decoded output. This calculation assumes the input length is a multiple of 4, but after filtering non-Base64 characters, the actual valid Base64 string length may differ. The allocation should be based on the filtered length l after line 108, not the original input length. This could lead to buffer overflow when writing decoded bytes. Consider moving the allocation after filtering, or allocate a safe upper bound like len bytes.

Copilot uses AI. Check for mistakes.
if (!dest) return NULL;

if (c3 != '=') {
*p++ = (((b2 & 0xf) << 4) | (b3 >> 2) );
}

if (c4 != '=') {
*p++ = (((b3 & 0x3) << 6) | b4 );
}
unsigned char *buf = (unsigned char *)malloc(len);
if (!buf) {
free(dest);
return NULL;
}

// Ignore non-Base64 characters
int l = 0;
for (int k = 0; k < len; k++) {
if (is_base64(src[k])) {
buf[l++] = src[k];
}

free(buf);

return (dest);
}
return (NULL);
}

static char encode(unsigned char u)
{
if (u < 26) {
return 'A' + u;
}
if (u < 52) {
return 'a' + (u - 26);
}
if (u < 62) {
return '0' + (u - 52);
}
if (u == 62) {
return '+';
}
unsigned char *p = (unsigned char *)dest;

return '/';
}
for (int k = 0; k < l; k += 4) {
unsigned char b1 = decode_char(buf[k]);
unsigned char b2 = decode_char((k + 1 < l) ? buf[k + 1] : 'A');
unsigned char b3 = decode_char((k + 2 < l) ? buf[k + 2] : 'A');
unsigned char b4 = decode_char((k + 3 < l) ? buf[k + 3] : 'A');

/* Decode a base64 character */
static unsigned char decode(char c)
{
if (c >= 'A' && c <= 'Z') {
return (c - 'A');
}
if (c >= 'a' && c <= 'z') {
return (c - 'a' + 26);
}
if (c >= '0' && c <= '9') {
return (c - '0' + 52);
*p++ = (b1 << 2) | (b2 >> 4);
if (buf[k + 2] != '=') *p++ = ((b2 & 0xf) << 4) | (b3 >> 2);
if (buf[k + 3] != '=') *p++ = ((b3 & 0x3) << 6) | b4;
Comment on lines +119 to +120
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The padding checks on lines 119-120 access buf[k + 2] and buf[k + 3] without verifying these indices are within bounds. If the filtered buffer length l is not a multiple of 4, these accesses could read beyond the allocated buffer, causing undefined behavior. You should add bounds checks: if (k + 2 < l && buf[k + 2] != '=') and if (k + 3 < l && buf[k + 3] != '=') before accessing these elements.

Suggested change
if (buf[k + 2] != '=') *p++ = ((b2 & 0xf) << 4) | (b3 >> 2);
if (buf[k + 3] != '=') *p++ = ((b3 & 0x3) << 6) | b4;
if (k + 2 < l && buf[k + 2] != '=') *p++ = ((b2 & 0xf) << 4) | (b3 >> 2);
if (k + 3 < l && buf[k + 3] != '=') *p++ = ((b3 & 0x3) << 6) | b4;

Copilot uses AI. Check for mistakes.
}
if (c == '+') {
return 62;
}

return 63;
}

Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to encode_base64, the null terminator is not explicitly written after decoding completes. While calloc initializes the buffer to zero, explicitly writing *p = '\0'; after the loop would make the null termination guarantee more obvious and maintainable. This is especially important for decode_base64 since the output length varies based on padding.

Suggested change
*p = '\0';

Copilot uses AI. Check for mistakes.
/* Returns TRUE if 'c' is a valid base64 character, otherwise FALSE */
static int is_base64(char c)
{
if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') ||
(c >= '0' && c <= '9') || (c == '+') ||
(c == '/') || (c == '=')) {

return TRUE;
}
return FALSE;
free(buf);
return dest;
}