Skip to content

Commit

Permalink
fix: malformed header crashes the bg worker
Browse files Browse the repository at this point in the history
- Now libcurl >= 7.83 is a requirement
- Tests are changed to fit new libcurl behavior
  • Loading branch information
steve-chavez committed Aug 9, 2024
1 parent 5a66e01 commit c2ba87b
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 70 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# PG_NET
*A PostgreSQL extension that enables asynchronous (non-blocking) HTTP/HTTPS requests with SQL*
*A PostgreSQL extension that enables asynchronous (non-blocking) HTTP/HTTPS requests with SQL*.

Requires libcurl >= 7.83.

![PostgreSQL version](https://img.shields.io/badge/postgresql-12+-blue.svg)
[![License](https://img.shields.io/pypi/l/markdown-subtemplate.svg)](https://github.com/supabase/pg_net/blob/master/LICENSE)
Expand Down
4 changes: 4 additions & 0 deletions nix/nginx/conf/custom.conf
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,7 @@ location /redirect_me {
location /to_here {
echo 'I got redirected';
}

location /pathological {
pathological;
}
16 changes: 15 additions & 1 deletion nix/nginxScript.nix
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
{ nginx, nginxModules, writeShellScriptBin } :
{ lib, fetchFromGitHub, nginx, nginxModules, writeShellScriptBin } :

let
ngx_pathological = rec {
name = "ngx_pathological";
version = "0.1";
src = fetchFromGitHub {
owner = "steve-chavez";
repo = name;
rev = "46a6d48910b482cf82ecd8a3fce448498cc9f886";
sha256 = "sha256-unPSFxbuvv5chyPKDRaKvxuekz3tPmhHiVzZzX1D4lk=";
};
meta = with lib; {
license = with licenses; [ mit ];
};
};
customNginx = nginx.override {
modules = [
nginxModules.echo
ngx_pathological
];
};
script = ''
Expand Down
17 changes: 17 additions & 0 deletions nix/ngx_pathological.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{ stdenv, postgresql, curl }:

stdenv.mkDerivation {
name = "pg_net";

buildInputs = [ postgresql curl ];

src = ../.;

installPhase = ''
mkdir -p $out/bin
install -D pg_net.so -t $out/lib
install -D -t $out/share/postgresql/extension sql/*.sql
install -D -t $out/share/postgresql/extension pg_net.control
'';
}
33 changes: 0 additions & 33 deletions src/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,36 +87,3 @@ struct curl_slist *pg_text_array_to_slist(ArrayType *array,

return headers;
}

/*TODO: make the parsing more robust, test with invalid headers*/
void parseHeaders(char *contents, JsonbParseState *headers) {
/* per curl docs, the status code is included in the header data
* (it starts with: HTTP/1.1 200 OK or HTTP/2 200 OK)*/
bool firstLine = strncmp(contents, "HTTP/", 5) == 0;
/* the final(end of headers) last line is empty - just a CRLF */
bool lastLine = strncmp(contents, "\r\n", 2) == 0;
char *token;
char *tmp = pstrdup(contents);
JsonbValue key, val;

if (!firstLine && !lastLine) {
/*The header comes as "Header-Key: val", split by whitespace and
ditch
* the colon later*/
token = strtok(tmp, " ");

key.type = jbvString;
key.val.string.val = token;
/*strlen - 1 because we ditch the last char - the colon*/
key.val.string.len = strlen(token) - 1;
(void)pushJsonbValue(&headers, WJB_KEY, &key);

/*Every header line ends with CRLF, split and remove it*/
token = strtok(NULL, "\r\n");

val.type = jbvString;
val.val.string.val = token;
val.val.string.len = strlen(token);
(void)pushJsonbValue(&headers, WJB_VALUE, &val);
}
}
3 changes: 0 additions & 3 deletions src/util.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#ifndef UTIL_H
#define UTIL_H

#include <utils/jsonb.h>

#define CURL_EZ_SETOPT(hdl, opt, prm) \
do { \
if (curl_easy_setopt(hdl, opt, prm) != CURLE_OK) \
Expand All @@ -24,6 +22,5 @@

extern struct curl_slist *pg_text_array_to_slist(ArrayType *array,
struct curl_slist *headers);
extern void parseHeaders(char *contents, JsonbParseState *headers);

#endif
59 changes: 27 additions & 32 deletions src/worker.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ PGDLLEXPORT void pg_net_worker(Datum main_arg) pg_attribute_noreturn();
typedef struct {
int64 id;
StringInfo body;
JsonbParseState* response_headers;
struct curl_slist* request_headers;
} CurlData;

Expand Down Expand Up @@ -73,26 +72,6 @@ body_cb(void *contents, size_t size, size_t nmemb, void *userp)
return realsize;
}

static size_t
header_cb(void *contents, size_t size, size_t nmemb, void *userp)
{
size_t realsize = size * nmemb;
JsonbParseState *headers = (JsonbParseState *)userp;

/* per curl docs, the status code is included in the header data
* (it starts with: HTTP/1.1 200 OK or HTTP/2 200 OK)*/
bool firstLine = strncmp(contents, "HTTP/", 5) == 0;
/* the final(end of headers) last line is empty - just a CRLF */
bool lastLine = strncmp(contents, "\r\n", 2) == 0;

/*Ignore non-header data in the first header line and last header line*/
if (!firstLine && !lastLine) {
parseHeaders(contents, headers);
}

return realsize;
}

static bool is_extension_loaded(){
Oid extensionOid;

Expand Down Expand Up @@ -142,7 +121,7 @@ static void insert_failure_response(CURLcode return_code, int64 id){
}
}

static void insert_success_response(CurlData *cdata, long http_status_code, char *contentType){
static void insert_success_response(CurlData *cdata, long http_status_code, char *contentType, Jsonb *jsonb_headers){
int ret_code = SPI_execute_with_args("\
insert into net._http_response(id, status_code, content, headers, content_type, timed_out) values ($1, $2, $3, $4, $5, $6)",
6,
Expand All @@ -151,7 +130,7 @@ static void insert_success_response(CurlData *cdata, long http_status_code, char
Int64GetDatum(cdata->id)
, Int32GetDatum(http_status_code)
, CStringGetDatum(cdata->body->data)
, JsonbPGetDatum(JsonbValueToJsonb(pushJsonbValue(&cdata->response_headers, WJB_END_OBJECT, NULL)))
, JsonbPGetDatum(jsonb_headers)
, CStringGetDatum(contentType)
// TODO Why is this hardcoded?
, BoolGetDatum(false)
Expand Down Expand Up @@ -205,8 +184,6 @@ static void init_curl_handle(CURLM *curl_mhandle, CurlData *cdata, char *url, ch

CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_WRITEFUNCTION, body_cb);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_WRITEDATA, cdata->body);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_HEADERFUNCTION, header_cb);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_HEADERDATA, cdata->response_headers);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_HEADER, 0L);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_URL, url);
CURL_EZ_SETOPT(curl_ez_handle, CURLOPT_HTTPHEADER, cdata->request_headers);
Expand Down Expand Up @@ -280,11 +257,8 @@ static void consume_request_queue(CURLM *curl_mhandle){
Datum bodyBin = SPI_getbinval(SPI_tuptable->vals[j], SPI_tuptable->tupdesc, 6, &tupIsNull);
if (!tupIsNull) reqBody = TextDatumGetCString(bodyBin);

JsonbParseState *response_headers = NULL;

cdata->body = makeStringInfo();
(void)pushJsonbValue(&response_headers, WJB_BEGIN_OBJECT, NULL);
cdata->response_headers = response_headers;
cdata->id = id;

struct curl_slist *new_headers = curl_slist_append(request_headers, "User-Agent: pg_net/" EXTVERSION);
Expand All @@ -297,6 +271,25 @@ static void consume_request_queue(CURLM *curl_mhandle){
}
}

static Jsonb *jsonb_headers_from_curl_handle(CURL *ez_handle){
struct curl_header *header, *prev = NULL;

JsonbParseState *headers = NULL;
(void)pushJsonbValue(&headers, WJB_BEGIN_OBJECT, NULL);

while((header = curl_easy_nextheader(ez_handle, CURLH_HEADER, 0, prev))) {
JsonbValue key = {.type = jbvString, .val = {.string = {.val = header->name, .len = strlen(header->name)}}};
JsonbValue value = {.type = jbvString, .val = {.string = {.val = header->value, .len = strlen(header->value)}}};
(void)pushJsonbValue(&headers, WJB_KEY, &key);
(void)pushJsonbValue(&headers, WJB_VALUE, &value);
prev = header;
}

Jsonb *jsonb_headers = JsonbValueToJsonb(pushJsonbValue(&headers, WJB_END_OBJECT, NULL));

return jsonb_headers;
}

static void insert_curl_responses(CURLM *curl_mhandle){
int still_running = 0;

Expand Down Expand Up @@ -331,13 +324,15 @@ static void insert_curl_responses(CURLM *curl_mhandle){
if (return_code != CURLE_OK) {
insert_failure_response(return_code, cdata->id);
} else {
char *contentType = NULL;
long http_status_code;
char *contentType;
CURL_EZ_GETINFO(ez_handle, CURLINFO_CONTENT_TYPE, &contentType);

long http_status_code;
CURL_EZ_GETINFO(ez_handle, CURLINFO_RESPONSE_CODE, &http_status_code);
CURL_EZ_GETINFO(ez_handle, CURLINFO_CONTENT_TYPE, &contentType);

insert_success_response(cdata, http_status_code, contentType);
Jsonb *jsonb_headers = jsonb_headers_from_curl_handle(ez_handle);

insert_success_response(cdata, http_status_code, contentType, jsonb_headers);

pfree_curl_data(cdata);
}
Expand Down
112 changes: 112 additions & 0 deletions test/test_http_malformed_headers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
from sqlalchemy import text

def test_http_header_missing_value(sess):
"""Check that a `MissingValue: ` header is processed correctly"""

(request_id,) = sess.execute(text(
"""
select net.http_get(
url:='http://localhost:8080/pathological?malformed-header=missing-value'
);
"""
)).fetchone()

# Commit so background worker can start
sess.commit()

# Collect the response, waiting as needed
response = sess.execute(
text(
"""
select * from net._http_collect_response(:request_id, async:=false);
"""
),
{"request_id": request_id},
).fetchone()
assert response is not None
assert response[0] == "SUCCESS"
assert "MissingValue" in response[2]


def test_http_header_injection(sess):
"""Check that a `HeaderInjection Injected-Header: This header contains an injection` header fails without crashing"""

(request_id,) = sess.execute(text(
"""
select net.http_get(
url:='http://localhost:8080/pathological?malformed-header=header-injection'
);
"""
)).fetchone()

# Commit so background worker can start
sess.commit()

# Collect the response, waiting as needed
response = sess.execute(
text(
"""
select * from net._http_collect_response(:request_id, async:=false);
"""
),
{"request_id": request_id},
).fetchone()
assert response is not None
assert response[0] == "ERROR"
assert "Weird server reply" in response[1]


def test_http_header_spaces(sess):
"""Check that a `Spaces In Header Name: This header name contains spaces` header is processed correctly"""

(request_id,) = sess.execute(text(
"""
select net.http_get(
url:='http://localhost:8080/pathological?malformed-header=spaces-in-header-name'
);
"""
)).fetchone()

# Commit so background worker can start
sess.commit()

# Collect the response, waiting as needed
response = sess.execute(
text(
"""
select * from net._http_collect_response(:request_id, async:=false);
"""
),
{"request_id": request_id},
).fetchone()
assert response is not None
assert response[0] == "SUCCESS"
assert "Spaces In Header Name" in response[2]


def test_http_header_non_printable_chars(sess):
"""Check that a `NonPrintableChars: NonPrintableChars\\u0001\\u0002` header is processed correctly"""

(request_id,) = sess.execute(text(
"""
select net.http_get(
url:='http://localhost:8080/pathological?malformed-header=non-printable-chars'
);
"""
)).fetchone()

# Commit so background worker can start
sess.commit()

# Collect the response, waiting as needed
response = sess.execute(
text(
"""
select * from net._http_collect_response(:request_id, async:=false);
"""
),
{"request_id": request_id},
).fetchone()
assert response is not None
assert response[0] == "SUCCESS"
assert r"NonPrintableChars\\u0001\\u0002" in response[2]

0 comments on commit c2ba87b

Please sign in to comment.