Fossil SCM
Slightly stronger detection of XSS attacks. Mostly this is rewording of comments to better explain how the anti-attack logic works.
Commit
8612122f501ba5407d2a8c02381994da4c324a480bb6aa764cd1383e31f80d14
Parent
5a33f307504dd68…
2 files changed
+13
-11
+13
-14
+13
-11
| --- src/cgi.c | ||
| +++ src/cgi.c | ||
| @@ -103,12 +103,12 @@ | ||
| 103 | 103 | #define PT(x) cgi_parameter_trimmed((x),0) |
| 104 | 104 | #define PDT(x,y) cgi_parameter_trimmed((x),(y)) |
| 105 | 105 | #define PB(x) cgi_parameter_boolean(x) |
| 106 | 106 | #define PCK(x) cgi_parameter_checked(x,1) |
| 107 | 107 | #define PIF(x,y) cgi_parameter_checked(x,y) |
| 108 | -#define P_NoBot(x) cgi_parameter_nosql((x),0) | |
| 109 | -#define PD_NoBot(x,y) cgi_parameter_nosql((x),(y)) | |
| 108 | +#define P_NoBot(x) cgi_parameter_no_attack((x),0) | |
| 109 | +#define PD_NoBot(x,y) cgi_parameter_no_attack((x),(y)) | |
| 110 | 110 | |
| 111 | 111 | /* |
| 112 | 112 | ** Shortcut for the cgi_printf() routine. Instead of using the |
| 113 | 113 | ** |
| 114 | 114 | ** @ ... |
| @@ -1620,37 +1620,39 @@ | ||
| 1620 | 1620 | fossil_errorlog("Xpossible hack attempt - 418 response on \"%s\"", zName); |
| 1621 | 1621 | exit(0); |
| 1622 | 1622 | } |
| 1623 | 1623 | |
| 1624 | 1624 | /* |
| 1625 | -** If looks_like_sql_injection() returns true for the given string, calls | |
| 1625 | +** If looks_like_attack() returns true for the given string, call | |
| 1626 | 1626 | ** cgi_begone_spider() and does not return, else this function has no |
| 1627 | 1627 | ** side effects. The range of checks performed by this function may |
| 1628 | 1628 | ** be extended in the future. |
| 1629 | 1629 | ** |
| 1630 | 1630 | ** Checks are omitted for any logged-in user. |
| 1631 | 1631 | ** |
| 1632 | -** This is NOT a defense against SQL injection. Fossil should easily be | |
| 1633 | -** proof against SQL injection without this routine. Rather, this is an | |
| 1634 | -** attempt to avoid denial-of-service caused by persistent spiders that hammer | |
| 1635 | -** the server with dozens or hundreds of SQL injection attempts per second | |
| 1636 | -** against pages (such as /vdiff) that are expensive to compute. In other | |
| 1632 | +** This is the primary defense against attack. Fossil should easily be | |
| 1633 | +** proof against SQL injection and XSS attacks even without without this | |
| 1634 | +** routine. Rather, this is an attempt to avoid denial-of-service caused | |
| 1635 | +** by persistent spiders that hammer the server with dozens or hundreds of | |
| 1636 | +** probes per seconds as they look for vulnerabilities. In other | |
| 1637 | 1637 | ** words, this is an effort to reduce the CPU load imposed by malicious |
| 1638 | -** spiders. It is not an effect defense against SQL injection vulnerabilities. | |
| 1638 | +** spiders. Though those routine might help make attacks harder, it is | |
| 1639 | +** not itself an impenetrably barrier against attack and should not be | |
| 1640 | +** relied upon as the only defense. | |
| 1639 | 1641 | */ |
| 1640 | 1642 | void cgi_value_spider_check(const char *zTxt, const char *zName){ |
| 1641 | - if( g.zLogin==0 && looks_like_sql_injection(zTxt) ){ | |
| 1643 | + if( g.zLogin==0 && looks_like_attack(zTxt) ){ | |
| 1642 | 1644 | cgi_begone_spider(zName); |
| 1643 | 1645 | } |
| 1644 | 1646 | } |
| 1645 | 1647 | |
| 1646 | 1648 | /* |
| 1647 | 1649 | ** A variant of cgi_parameter() with the same semantics except that if |
| 1648 | 1650 | ** cgi_parameter(zName,zDefault) returns a value other than zDefault |
| 1649 | 1651 | ** then it passes that value to cgi_value_spider_check(). |
| 1650 | 1652 | */ |
| 1651 | -const char *cgi_parameter_nosql(const char *zName, const char *zDefault){ | |
| 1653 | +const char *cgi_parameter_no_attack(const char *zName, const char *zDefault){ | |
| 1652 | 1654 | const char *zTxt = cgi_parameter(zName, zDefault); |
| 1653 | 1655 | |
| 1654 | 1656 | if( zTxt!=zDefault ){ |
| 1655 | 1657 | cgi_value_spider_check(zTxt, zName); |
| 1656 | 1658 | } |
| 1657 | 1659 |
| --- src/cgi.c | |
| +++ src/cgi.c | |
| @@ -103,12 +103,12 @@ | |
| 103 | #define PT(x) cgi_parameter_trimmed((x),0) |
| 104 | #define PDT(x,y) cgi_parameter_trimmed((x),(y)) |
| 105 | #define PB(x) cgi_parameter_boolean(x) |
| 106 | #define PCK(x) cgi_parameter_checked(x,1) |
| 107 | #define PIF(x,y) cgi_parameter_checked(x,y) |
| 108 | #define P_NoBot(x) cgi_parameter_nosql((x),0) |
| 109 | #define PD_NoBot(x,y) cgi_parameter_nosql((x),(y)) |
| 110 | |
| 111 | /* |
| 112 | ** Shortcut for the cgi_printf() routine. Instead of using the |
| 113 | ** |
| 114 | ** @ ... |
| @@ -1620,37 +1620,39 @@ | |
| 1620 | fossil_errorlog("Xpossible hack attempt - 418 response on \"%s\"", zName); |
| 1621 | exit(0); |
| 1622 | } |
| 1623 | |
| 1624 | /* |
| 1625 | ** If looks_like_sql_injection() returns true for the given string, calls |
| 1626 | ** cgi_begone_spider() and does not return, else this function has no |
| 1627 | ** side effects. The range of checks performed by this function may |
| 1628 | ** be extended in the future. |
| 1629 | ** |
| 1630 | ** Checks are omitted for any logged-in user. |
| 1631 | ** |
| 1632 | ** This is NOT a defense against SQL injection. Fossil should easily be |
| 1633 | ** proof against SQL injection without this routine. Rather, this is an |
| 1634 | ** attempt to avoid denial-of-service caused by persistent spiders that hammer |
| 1635 | ** the server with dozens or hundreds of SQL injection attempts per second |
| 1636 | ** against pages (such as /vdiff) that are expensive to compute. In other |
| 1637 | ** words, this is an effort to reduce the CPU load imposed by malicious |
| 1638 | ** spiders. It is not an effect defense against SQL injection vulnerabilities. |
| 1639 | */ |
| 1640 | void cgi_value_spider_check(const char *zTxt, const char *zName){ |
| 1641 | if( g.zLogin==0 && looks_like_sql_injection(zTxt) ){ |
| 1642 | cgi_begone_spider(zName); |
| 1643 | } |
| 1644 | } |
| 1645 | |
| 1646 | /* |
| 1647 | ** A variant of cgi_parameter() with the same semantics except that if |
| 1648 | ** cgi_parameter(zName,zDefault) returns a value other than zDefault |
| 1649 | ** then it passes that value to cgi_value_spider_check(). |
| 1650 | */ |
| 1651 | const char *cgi_parameter_nosql(const char *zName, const char *zDefault){ |
| 1652 | const char *zTxt = cgi_parameter(zName, zDefault); |
| 1653 | |
| 1654 | if( zTxt!=zDefault ){ |
| 1655 | cgi_value_spider_check(zTxt, zName); |
| 1656 | } |
| 1657 |
| --- src/cgi.c | |
| +++ src/cgi.c | |
| @@ -103,12 +103,12 @@ | |
| 103 | #define PT(x) cgi_parameter_trimmed((x),0) |
| 104 | #define PDT(x,y) cgi_parameter_trimmed((x),(y)) |
| 105 | #define PB(x) cgi_parameter_boolean(x) |
| 106 | #define PCK(x) cgi_parameter_checked(x,1) |
| 107 | #define PIF(x,y) cgi_parameter_checked(x,y) |
| 108 | #define P_NoBot(x) cgi_parameter_no_attack((x),0) |
| 109 | #define PD_NoBot(x,y) cgi_parameter_no_attack((x),(y)) |
| 110 | |
| 111 | /* |
| 112 | ** Shortcut for the cgi_printf() routine. Instead of using the |
| 113 | ** |
| 114 | ** @ ... |
| @@ -1620,37 +1620,39 @@ | |
| 1620 | fossil_errorlog("Xpossible hack attempt - 418 response on \"%s\"", zName); |
| 1621 | exit(0); |
| 1622 | } |
| 1623 | |
| 1624 | /* |
| 1625 | ** If looks_like_attack() returns true for the given string, call |
| 1626 | ** cgi_begone_spider() and does not return, else this function has no |
| 1627 | ** side effects. The range of checks performed by this function may |
| 1628 | ** be extended in the future. |
| 1629 | ** |
| 1630 | ** Checks are omitted for any logged-in user. |
| 1631 | ** |
| 1632 | ** This is the primary defense against attack. Fossil should easily be |
| 1633 | ** proof against SQL injection and XSS attacks even without without this |
| 1634 | ** routine. Rather, this is an attempt to avoid denial-of-service caused |
| 1635 | ** by persistent spiders that hammer the server with dozens or hundreds of |
| 1636 | ** probes per seconds as they look for vulnerabilities. In other |
| 1637 | ** words, this is an effort to reduce the CPU load imposed by malicious |
| 1638 | ** spiders. Though those routine might help make attacks harder, it is |
| 1639 | ** not itself an impenetrably barrier against attack and should not be |
| 1640 | ** relied upon as the only defense. |
| 1641 | */ |
| 1642 | void cgi_value_spider_check(const char *zTxt, const char *zName){ |
| 1643 | if( g.zLogin==0 && looks_like_attack(zTxt) ){ |
| 1644 | cgi_begone_spider(zName); |
| 1645 | } |
| 1646 | } |
| 1647 | |
| 1648 | /* |
| 1649 | ** A variant of cgi_parameter() with the same semantics except that if |
| 1650 | ** cgi_parameter(zName,zDefault) returns a value other than zDefault |
| 1651 | ** then it passes that value to cgi_value_spider_check(). |
| 1652 | */ |
| 1653 | const char *cgi_parameter_no_attack(const char *zName, const char *zDefault){ |
| 1654 | const char *zTxt = cgi_parameter(zName, zDefault); |
| 1655 | |
| 1656 | if( zTxt!=zDefault ){ |
| 1657 | cgi_value_spider_check(zTxt, zName); |
| 1658 | } |
| 1659 |
+13
-14
| --- src/lookslike.c | ||
| +++ src/lookslike.c | ||
| @@ -478,25 +478,24 @@ | ||
| 478 | 478 | } |
| 479 | 479 | |
| 480 | 480 | /* |
| 481 | 481 | ** Returns true if the given text contains certain keywords or |
| 482 | 482 | ** punctuation which indicate that it might be an SQL injection attempt |
| 483 | -** or some other kind of mischief. | |
| 483 | +** or Cross-site scripting attempt or some other kind of mischief. | |
| 484 | 484 | ** |
| 485 | -** This is not a defense against vulnerabilities in the Fossil code. | |
| 486 | -** Rather, this is part of an effort to do early detection of malicious | |
| 487 | -** spiders to avoid them using up too many CPU cycles. | |
| 485 | +** This is not a primary defense against vulnerabilities in the Fossil | |
| 486 | +** code. Rather, this is part of an effort to do early detection of malicious | |
| 487 | +** spiders to avoid them using up too many CPU cycles. Or, this routine | |
| 488 | +** can also be thought of as a secondary layer of defense against attacks. | |
| 488 | 489 | */ |
| 489 | -int looks_like_sql_injection(const char *zTxt){ | |
| 490 | +int looks_like_attack(const char *zTxt){ | |
| 490 | 491 | unsigned int i; |
| 491 | 492 | int rc = 0; |
| 492 | 493 | if( zTxt==0 ) return 0; |
| 493 | 494 | for(i=0; zTxt[i]; i++){ |
| 494 | 495 | switch( zTxt[i] ){ |
| 495 | 496 | case '<': |
| 496 | - if( sqlite3_strnicmp(zTxt+i, "<script>", 8)==0 ) rc = 1; | |
| 497 | - break; | |
| 498 | 497 | case ';': |
| 499 | 498 | case '\'': |
| 500 | 499 | return 1; |
| 501 | 500 | case '/': /* 0123456789 123456789 */ |
| 502 | 501 | if( strncmp(zTxt+i+1, "/wp-content/plugins/", 20)==0 ) rc = 1; |
| @@ -547,11 +546,11 @@ | ||
| 547 | 546 | ** might be SQL injection. |
| 548 | 547 | ** |
| 549 | 548 | ** Or if bInvert is true, then show the opposite - those lines that do NOT |
| 550 | 549 | ** look like SQL injection. |
| 551 | 550 | */ |
| 552 | -static void show_sql_injection_lines( | |
| 551 | +static void show_attack_lines( | |
| 553 | 552 | const char *zInFile, /* Name of input file */ |
| 554 | 553 | int bInvert, /* Invert the sense of the output (-v) */ |
| 555 | 554 | int bDeHttpize /* De-httpize the inputs. (-d) */ |
| 556 | 555 | ){ |
| 557 | 556 | FILE *in; |
| @@ -564,34 +563,34 @@ | ||
| 564 | 563 | fossil_fatal("cannot open \"%s\" for reading\n", zInFile); |
| 565 | 564 | } |
| 566 | 565 | } |
| 567 | 566 | while( fgets(zLine, sizeof(zLine), in) ){ |
| 568 | 567 | dehttpize(zLine); |
| 569 | - if( (looks_like_sql_injection(zLine)!=0) ^ bInvert ){ | |
| 568 | + if( (looks_like_attack(zLine)!=0) ^ bInvert ){ | |
| 570 | 569 | fossil_print("%s", zLine); |
| 571 | 570 | } |
| 572 | 571 | } |
| 573 | 572 | if( in!=stdin ) fclose(in); |
| 574 | 573 | } |
| 575 | 574 | |
| 576 | 575 | /* |
| 577 | -** COMMAND: test-looks-like-sql-injection | |
| 576 | +** COMMAND: test-looks-like-attack | |
| 578 | 577 | ** |
| 579 | 578 | ** Read lines of input from files named as arguments (or from standard |
| 580 | 579 | ** input if no arguments are provided) and print those that look like they |
| 581 | 580 | ** might be part of an SQL injection attack. |
| 582 | 581 | ** |
| 583 | -** Used to test the looks_lide_sql_injection() utility subroutine, possibly | |
| 582 | +** Used to test the looks_lile_attack() utility subroutine, possibly | |
| 584 | 583 | ** by piping in actual server log data. |
| 585 | 584 | */ |
| 586 | -void test_looks_like_sql_injection(void){ | |
| 585 | +void test_looks_like_attack(void){ | |
| 587 | 586 | int i; |
| 588 | 587 | int bInvert = find_option("invert","v",0)!=0; |
| 589 | 588 | int bDeHttpize = find_option("dehttpize","d",0)!=0; |
| 590 | 589 | verify_all_options(); |
| 591 | 590 | if( g.argc==2 ){ |
| 592 | - show_sql_injection_lines(0, bInvert, bDeHttpize); | |
| 591 | + show_attack_lines(0, bInvert, bDeHttpize); | |
| 593 | 592 | } |
| 594 | 593 | for(i=2; i<g.argc; i++){ |
| 595 | - show_sql_injection_lines(g.argv[i], bInvert, bDeHttpize); | |
| 594 | + show_attack_lines(g.argv[i], bInvert, bDeHttpize); | |
| 596 | 595 | } |
| 597 | 596 | } |
| 598 | 597 |
| --- src/lookslike.c | |
| +++ src/lookslike.c | |
| @@ -478,25 +478,24 @@ | |
| 478 | } |
| 479 | |
| 480 | /* |
| 481 | ** Returns true if the given text contains certain keywords or |
| 482 | ** punctuation which indicate that it might be an SQL injection attempt |
| 483 | ** or some other kind of mischief. |
| 484 | ** |
| 485 | ** This is not a defense against vulnerabilities in the Fossil code. |
| 486 | ** Rather, this is part of an effort to do early detection of malicious |
| 487 | ** spiders to avoid them using up too many CPU cycles. |
| 488 | */ |
| 489 | int looks_like_sql_injection(const char *zTxt){ |
| 490 | unsigned int i; |
| 491 | int rc = 0; |
| 492 | if( zTxt==0 ) return 0; |
| 493 | for(i=0; zTxt[i]; i++){ |
| 494 | switch( zTxt[i] ){ |
| 495 | case '<': |
| 496 | if( sqlite3_strnicmp(zTxt+i, "<script>", 8)==0 ) rc = 1; |
| 497 | break; |
| 498 | case ';': |
| 499 | case '\'': |
| 500 | return 1; |
| 501 | case '/': /* 0123456789 123456789 */ |
| 502 | if( strncmp(zTxt+i+1, "/wp-content/plugins/", 20)==0 ) rc = 1; |
| @@ -547,11 +546,11 @@ | |
| 547 | ** might be SQL injection. |
| 548 | ** |
| 549 | ** Or if bInvert is true, then show the opposite - those lines that do NOT |
| 550 | ** look like SQL injection. |
| 551 | */ |
| 552 | static void show_sql_injection_lines( |
| 553 | const char *zInFile, /* Name of input file */ |
| 554 | int bInvert, /* Invert the sense of the output (-v) */ |
| 555 | int bDeHttpize /* De-httpize the inputs. (-d) */ |
| 556 | ){ |
| 557 | FILE *in; |
| @@ -564,34 +563,34 @@ | |
| 564 | fossil_fatal("cannot open \"%s\" for reading\n", zInFile); |
| 565 | } |
| 566 | } |
| 567 | while( fgets(zLine, sizeof(zLine), in) ){ |
| 568 | dehttpize(zLine); |
| 569 | if( (looks_like_sql_injection(zLine)!=0) ^ bInvert ){ |
| 570 | fossil_print("%s", zLine); |
| 571 | } |
| 572 | } |
| 573 | if( in!=stdin ) fclose(in); |
| 574 | } |
| 575 | |
| 576 | /* |
| 577 | ** COMMAND: test-looks-like-sql-injection |
| 578 | ** |
| 579 | ** Read lines of input from files named as arguments (or from standard |
| 580 | ** input if no arguments are provided) and print those that look like they |
| 581 | ** might be part of an SQL injection attack. |
| 582 | ** |
| 583 | ** Used to test the looks_lide_sql_injection() utility subroutine, possibly |
| 584 | ** by piping in actual server log data. |
| 585 | */ |
| 586 | void test_looks_like_sql_injection(void){ |
| 587 | int i; |
| 588 | int bInvert = find_option("invert","v",0)!=0; |
| 589 | int bDeHttpize = find_option("dehttpize","d",0)!=0; |
| 590 | verify_all_options(); |
| 591 | if( g.argc==2 ){ |
| 592 | show_sql_injection_lines(0, bInvert, bDeHttpize); |
| 593 | } |
| 594 | for(i=2; i<g.argc; i++){ |
| 595 | show_sql_injection_lines(g.argv[i], bInvert, bDeHttpize); |
| 596 | } |
| 597 | } |
| 598 |
| --- src/lookslike.c | |
| +++ src/lookslike.c | |
| @@ -478,25 +478,24 @@ | |
| 478 | } |
| 479 | |
| 480 | /* |
| 481 | ** Returns true if the given text contains certain keywords or |
| 482 | ** punctuation which indicate that it might be an SQL injection attempt |
| 483 | ** or Cross-site scripting attempt or some other kind of mischief. |
| 484 | ** |
| 485 | ** This is not a primary defense against vulnerabilities in the Fossil |
| 486 | ** code. Rather, this is part of an effort to do early detection of malicious |
| 487 | ** spiders to avoid them using up too many CPU cycles. Or, this routine |
| 488 | ** can also be thought of as a secondary layer of defense against attacks. |
| 489 | */ |
| 490 | int looks_like_attack(const char *zTxt){ |
| 491 | unsigned int i; |
| 492 | int rc = 0; |
| 493 | if( zTxt==0 ) return 0; |
| 494 | for(i=0; zTxt[i]; i++){ |
| 495 | switch( zTxt[i] ){ |
| 496 | case '<': |
| 497 | case ';': |
| 498 | case '\'': |
| 499 | return 1; |
| 500 | case '/': /* 0123456789 123456789 */ |
| 501 | if( strncmp(zTxt+i+1, "/wp-content/plugins/", 20)==0 ) rc = 1; |
| @@ -547,11 +546,11 @@ | |
| 546 | ** might be SQL injection. |
| 547 | ** |
| 548 | ** Or if bInvert is true, then show the opposite - those lines that do NOT |
| 549 | ** look like SQL injection. |
| 550 | */ |
| 551 | static void show_attack_lines( |
| 552 | const char *zInFile, /* Name of input file */ |
| 553 | int bInvert, /* Invert the sense of the output (-v) */ |
| 554 | int bDeHttpize /* De-httpize the inputs. (-d) */ |
| 555 | ){ |
| 556 | FILE *in; |
| @@ -564,34 +563,34 @@ | |
| 563 | fossil_fatal("cannot open \"%s\" for reading\n", zInFile); |
| 564 | } |
| 565 | } |
| 566 | while( fgets(zLine, sizeof(zLine), in) ){ |
| 567 | dehttpize(zLine); |
| 568 | if( (looks_like_attack(zLine)!=0) ^ bInvert ){ |
| 569 | fossil_print("%s", zLine); |
| 570 | } |
| 571 | } |
| 572 | if( in!=stdin ) fclose(in); |
| 573 | } |
| 574 | |
| 575 | /* |
| 576 | ** COMMAND: test-looks-like-attack |
| 577 | ** |
| 578 | ** Read lines of input from files named as arguments (or from standard |
| 579 | ** input if no arguments are provided) and print those that look like they |
| 580 | ** might be part of an SQL injection attack. |
| 581 | ** |
| 582 | ** Used to test the looks_lile_attack() utility subroutine, possibly |
| 583 | ** by piping in actual server log data. |
| 584 | */ |
| 585 | void test_looks_like_attack(void){ |
| 586 | int i; |
| 587 | int bInvert = find_option("invert","v",0)!=0; |
| 588 | int bDeHttpize = find_option("dehttpize","d",0)!=0; |
| 589 | verify_all_options(); |
| 590 | if( g.argc==2 ){ |
| 591 | show_attack_lines(0, bInvert, bDeHttpize); |
| 592 | } |
| 593 | for(i=2; i<g.argc; i++){ |
| 594 | show_attack_lines(g.argv[i], bInvert, bDeHttpize); |
| 595 | } |
| 596 | } |
| 597 |